diff --git a/core/src/global/ionic-global.ts b/core/src/global/ionic-global.ts index 2a4b49a93a7..e3ce11b1f4e 100644 --- a/core/src/global/ionic-global.ts +++ b/core/src/global/ionic-global.ts @@ -177,28 +177,38 @@ export const initialize = (userConfig: IonicConfig = {}) => { } /** - * Apply any global config values from the custom theme - * excluding component-specific overrides. + * Apply any global config values from the resolved theme + * excluding component-specific overrides. The resolved theme + * is the base theme merged with the custom theme, so a value + * from the custom theme takes precedence over the base theme. + * The theme only provides defaults, so any value set by the + * application takes precedence over the value from the theme. */ + const resolvedTheme: DefaultTheme | undefined = config.get('customTheme'); // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { components, ...customGlobalConfig } = customTheme?.config || {}; + const { components, ...themeGlobalConfig } = resolvedTheme?.config || {}; - Object.entries(customGlobalConfig).forEach(([key, value]) => { + const isPlainObject = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); + + Object.entries(themeGlobalConfig).forEach(([key, value]) => { const configKey = key as keyof IonicConfig; + const appValue = config.get(configKey); - // For object-based config values, we merge with existing settings. - if (value !== null && typeof value === 'object') { - const existingConfig = config.get(configKey, {}); - config.set(configKey, { ...existingConfig, ...value }); + // The theme value is used when the application has not set one. + if (appValue === undefined) { + config.set(configKey, value); return; } /** - * For primitive values (boolean, string, number), we set the - * value directly. - * This covers global flags like 'rippleEffect' or 'animated'. + * For object-based config values, such as 'screenBreakpoints', + * the theme fills in the keys the application has not set. + * Any other value set by the application is kept as is. */ - config.set(configKey, value); + if (isPlainObject(value) && isPlainObject(appValue)) { + config.set(configKey, { ...value, ...appValue }); + } }); /** diff --git a/core/src/global/test/ionic-global.spec.ts b/core/src/global/test/ionic-global.spec.ts index 25f431de538..1fce165aa06 100644 --- a/core/src/global/test/ionic-global.spec.ts +++ b/core/src/global/test/ionic-global.spec.ts @@ -138,9 +138,9 @@ describe('Ionic Global', () => { }); /** - * A theme can set the screen breakpoints in its own config block, which is - * merged in after the application config, so the listeners can only be - * rebuilt once both have been applied. + * A theme can set the screen breakpoints in its own config block, which + * provides defaults for the application config, so the listeners can only + * be rebuilt once both have been applied. */ it('should rebuild the breakpoint listeners from a theme that sets the widths', () => { // Subscribing first is what creates the listeners there are to rebuild. @@ -156,8 +156,8 @@ describe('Ionic Global', () => { }); /** - * The theme's config is merged over the application's, so widths set in - * either are kept and the listeners are built from the result. + * The theme's config fills in the widths the application has not set, so + * widths set in either are kept and the listeners are built from the result. */ it('should keep widths set by the application alongside those from the theme', () => { const unsubscribe = onBreakpointChange(() => {}); @@ -174,6 +174,69 @@ describe('Ionic Global', () => { unsubscribe(); }); + it('should use the application value over the theme value for a primitive config option', () => { + initialize({ + rippleEffect: false, + formHighlight: false, + customTheme: { config: { rippleEffect: true, formHighlight: true } }, + } as any); + + expect(config.get('rippleEffect')).toBe(false); + expect(config.get('formHighlight')).toBe(false); + }); + + it('should use the theme value for a config option the application has not set', () => { + initialize({ customTheme: { config: { rippleEffect: true } } } as any); + + expect(config.get('rippleEffect')).toBe(true); + }); + + it('should use the application value over the theme value for an overlapping object key', () => { + initialize({ + screenBreakpoints: { md: 500, lg: 900 }, + customTheme: { config: { screenBreakpoints: { md: 700, xl: 1300 } } }, + } as any); + + expect(config.get('screenBreakpoints')).toEqual({ md: 500, lg: 900, xl: 1300 }); + }); + + it('should keep an application value that is not an object when the theme value is', () => { + initialize({ + screenBreakpoints: 'invalid', + customTheme: { config: { screenBreakpoints: { md: 700 } } }, + } as any); + + expect(config.get('screenBreakpoints')).toBe('invalid'); + }); + + it('should apply the base theme config when no custom theme is provided', () => { + initialize(); + + expect(config.get('formHighlight')).toBe(false); + }); + + it('should use the custom theme value over the base theme value', () => { + initialize({ customTheme: { config: { formHighlight: true } } } as any); + + expect(config.get('formHighlight')).toBe(true); + }); + + it('should use the application value over the base theme value', () => { + initialize({ formHighlight: true } as any); + + expect(config.get('formHighlight')).toBe(true); + }); + + /** + * Ripple effects are enabled by default for the md theme through the + * tap-click fallback, so the base theme must leave the value unset. + */ + it('should not set the ripple effect when no custom theme is provided', () => { + initialize(); + + expect(config.get('rippleEffect')).toBeUndefined(); + }); + /** * The media queries are generated from the configured widths too, so they * have to be emitted once the theme's config has been merged as well. diff --git a/core/src/themes/base/default.tokens.ts b/core/src/themes/base/default.tokens.ts index 26906072d12..f9b6d8817be 100644 --- a/core/src/themes/base/default.tokens.ts +++ b/core/src/themes/base/default.tokens.ts @@ -17,7 +17,6 @@ export const defaultTheme: DefaultTheme = { config: { formHighlight: false, - rippleEffect: false, }, spacing: {