Fix merging of child / parent themes.

Author: akademy

changelog.md

@@ -10,6 +10,7 @@ SciReactUI Changelog
 
 ### Fixed
 - Styles added to Navbar and Footer incorrectly remove built in styles.
+- Themes were not inheriting all details from their parent.
 
 ### Changed
 - Breadcrumbs component takes optional linkComponent prop for page routing. 

src/storybook/theme/Colours.mdx

@@ -59,13 +59,13 @@ export function ThemeColorItem({title, theme, colourSet, mode}) {
             ### Light Mode
             <ColorPalette>
                 <ThemeColorItem title="Primary" theme={DiamondTheme} colourSet="primary" mode="light" />
-                <ThemeColorItem title="Secodary" theme={DiamondTheme} colourSet="secondary" mode="light" />
+                <ThemeColorItem title="Secondary" theme={DiamondTheme} colourSet="secondary" mode="light" />
             </ColorPalette>
 
             ### Dark Mode
             <ColorPalette>
                 <ThemeColorItem title="Primary" theme={DiamondTheme} colourSet="primary" mode={"dark"} />
-                <ThemeColorItem title="Secodary" theme={DiamondTheme} colourSet="secondary" mode={"dark"} />
+                <ThemeColorItem title="Secondary" theme={DiamondTheme} colourSet="secondary" mode={"dark"} />
             </ColorPalette>
         </div>
     </div>

src/storybook/theme/Logos.mdx

@@ -1,4 +1,4 @@
-import { Meta, ColorPalette, ColorItem } from '@storybook/blocks';
+import { Meta } from '@storybook/blocks';
 import {ImageColorSchemeSwitch} from "../../components/controls/ImageColorSchemeSwitch";
 
 <Meta title="Theme/Logos" />
@@ -24,7 +24,10 @@ import {ImageColorSchemeSwitch} from "../../components/controls/ImageColorScheme
         }
 		```
 
-        <ImageColorSchemeSwitch image={{"src":"static/media/src/public/generic/logo-light.svg","alt":"A Logo"}}/>
+        <div style={{width:"200px", margin:"0 auto"}} >
+            <ImageColorSchemeSwitch image={{"src":"static/media/src/public/generic/logo-light.svg","alt":"A Logo",width:200}}/>
+            <p>Example logo</p>
+        </div>
 
         ## Sizes
 		There are two image sizes. I regular `normal` and a smaller version `short`, these are used in the Navbar and

src/themes/BaseTheme.ts

@@ -29,11 +29,17 @@ const BaseThemeOptions /* : ThemeOptions */ = {
       palette: {
         background: { default: "#fafafa" },
       },
+      text: {
+        primary: "#050505"
+      },
     },
     dark: {
       palette: {
         background: { default: "#050505" },
       },
+      text: {
+        primary: "#fafafa"
+      },
     },
   },
   components: {},

src/themes/DiamondTheme.ts

@@ -1,7 +1,7 @@
-import { createTheme, Theme } from "@mui/material/styles";
 import type {} from "@mui/material/themeCssVarsAugmentation";
+import { createTheme, Theme } from "@mui/material/styles";
 
-import { BaseThemeOptions } from "./BaseTheme";
+import {mergeThemeOptions} from "./ThemeManager";
 
 import logoImageDark from "../public/diamond/logo-dark.svg";
 import logoImageLight from "../public/diamond/logo-light.svg";
@@ -10,8 +10,7 @@ import logoShort from "../public/diamond/logo-short.svg";
 const dlsLogoBlue = "#202740";
 const dlsLogoYellow = "#facf07";
 
-const DiamondTheme: Theme = createTheme({
-  ...BaseThemeOptions,
+const DiamondThemeOptions = mergeThemeOptions({
   logos: {
     normal: {
       src: logoImageLight,
@@ -41,6 +40,9 @@ const DiamondTheme: Theme = createTheme({
           dark: "#AF9004", // dark yellow
           contrastText: "#000000", // black
         },
+        text: {
+          secondary: "#161B2C"
+        },
       },
     },
     dark: {
@@ -57,13 +59,16 @@ const DiamondTheme: Theme = createTheme({
           dark: "#AF9004", // dark yellow
           contrastText: "#000000", // black
         },
+        text: {
+          secondary: "#8090CA"
+        },
       },
     },
   },
   components: {
     MuiButton: {
       styleOverrides: {
-        root: ({ theme }) => ({
+        root: ({ theme }: any ) => ({
           textTransform: "none",
           "&.MuiButton-contained": {},
           "&.default": {
@@ -92,4 +97,6 @@ const DiamondTheme: Theme = createTheme({
   },
 });
 
-export { DiamondTheme };
+const DiamondTheme: Theme = createTheme( DiamondThemeOptions );
+
+export { DiamondTheme, DiamondThemeOptions };

src/themes/GenericTheme.ts

@@ -1,12 +1,12 @@
-import { createTheme, Theme } from "@mui/material/styles";
+import type {} from "@mui/material/themeCssVarsAugmentation";
+import {createTheme, Theme} from "@mui/material/styles";
 
-import { BaseThemeOptions } from "./BaseTheme";
+import {mergeThemeOptions} from "./ThemeManager";
 
 import logoImageDark from "../public/generic/logo-dark.svg";
 import logoImageLight from "../public/generic/logo-light.svg";
 
-const GenericTheme: Theme = createTheme({
-  ...BaseThemeOptions,
+const GenericThemeOptions = mergeThemeOptions({
   logos: {
     normal: {
       src: logoImageLight,
@@ -22,4 +22,6 @@ const GenericTheme: Theme = createTheme({
   },
 });
 
-export { GenericTheme };
+const GenericTheme: Theme = createTheme(GenericThemeOptions)
+console.log( GenericTheme )
+export { GenericTheme, GenericThemeOptions };

src/themes/ThemeManager.test.ts

@@ -0,0 +1,50 @@
+import "@testing-library/jest-dom";
+
+import { BaseThemeOptions } from "./BaseTheme";
+import { mergeThemeOptions } from "./ThemeManager";
+
+describe("Theme Manager merge", () => {
+	it("should merge", () => {
+		const a = { a: 1, b: 2 };
+		const b = { x: 1, y: 2 };
+		const new_a = mergeThemeOptions(a,b);
+		
+		expect(new_a).toStrictEqual( { a: 1, b: 2, x: 1, y: 2} )
+	})
+	
+	
+	it("should deep merge", () => {
+		const a = { a: 1, b: 2, c: {d: 1, e: 2}};
+		const b = { x: 1, y:{ z: 3 } };
+		const merged = mergeThemeOptions(a, b);
+		
+		expect(merged).toStrictEqual( { a: 1, b: 2, c: {d: 1, e: 2}, x: 1, y:{ z: 3 }} )
+	})
+	
+	it("should use a value over b", () => {
+		const a = {a: 100, b: 2};
+		const b = {a: 1, c: 3};
+		const merged = mergeThemeOptions(a, b);
+
+		expect(merged).toStrictEqual({ a: 100, b: 2, c:3 })
+	})
+	
+	it("should take the base theme and make a new one", () => {
+		const fontSize = 4422;
+		const a = { typography: { fontSize: fontSize } };
+		const b = BaseThemeOptions;
+		
+		const merged = mergeThemeOptions(a, b);
+		
+		expect(BaseThemeOptions.typography.fontSize).not.toStrictEqual( fontSize )
+		expect(merged.typography.fontSize).toStrictEqual( fontSize )
+	})
+	
+	it("should effectively clone to an empty object", () => {
+		const a = {a: 100, b: 2};
+		const cloned = mergeThemeOptions({}, a);
+		
+		expect(cloned).toStrictEqual(a)
+	})
+	
+})

src/themes/ThemeManager.ts

@@ -0,0 +1,45 @@
+import { BaseThemeOptions } from "./BaseTheme";
+
+/*
+	Merge two options, with newThemeOptions having precedence.
+	If no parent is selected the BaseThemeOptions is used
+	Doesn't affect either options passed in.
+ */
+function mergeThemeOptions(newThemeOptions: any, parentThemeOptions: any=BaseThemeOptions ) {
+	const parentThemeOptionsCopy = deepCopyObject(parentThemeOptions);
+	return mergeObjects(parentThemeOptionsCopy, newThemeOptions )
+}
+
+
+function mergeObjects(mainThemeOptions: any, parentThemeOptions: any, visited = new Map<any, any>()) {
+	//This Deep Merge algorithm is based on https://www.geeksforgeeks.org/how-to-deep-merge-two-objects-in-typescript/
+	if (isObject(mainThemeOptions) && isObject(parentThemeOptions)) {
+		for (const key in parentThemeOptions) {
+			if (isObject(parentThemeOptions[key])) {
+				if (!mainThemeOptions[key]) {
+					mainThemeOptions[key] = {};
+				}
+				// Check if the parentThemeOptions object has already been visited
+				if (!visited.has(parentThemeOptions[key])) {
+					visited.set(parentThemeOptions[key], {});
+					mergeObjects(mainThemeOptions[key], parentThemeOptions[key], visited);
+				} else {
+					mainThemeOptions[key] = visited.get(parentThemeOptions[key]);
+				}
+			} else {
+				mainThemeOptions[key] = parentThemeOptions[key];
+			}
+		}
+	}
+	return mainThemeOptions;
+}
+
+function isObject(item: any): boolean {
+	return item !== null && typeof item === 'object' && !Array.isArray(item);
+}
+
+function deepCopyObject(themeOptions: any) {
+	return mergeObjects({}, themeOptions )
+}
+
+export { mergeThemeOptions };