Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the theme middleware to deal with the compose scenarios #940

Merged
merged 13 commits into from
Dec 3, 2019

Conversation

agubler
Copy link
Member

@agubler agubler commented Dec 2, 2019

Type: bug

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit tests are included in the PR
  • Any widget variant uses theme.compose like this
  • WidgetProperties are exported

Description:

Update theme middleware to support default variant themes without requiring the use of composes for the default base theme in the variant css.

  1. The combined classes from the variant css and base css are available for theming.
// base widget default theme
.root {

}

// variant widget default theme
.extra {

}

This will enable both the root and extra classes to be themed by targeting the variants key, for example:

// theme
{
    '@dojo/widgets/variant': {
        root: 'variant_theme_root',
        extra: 'variant_theme_extra'
    }
}
  1. Falls back to the base theme when not specified in the variants theme
// theme
{
    '@dojo/widgets/base': {
        root: 'base_theme_root'
    },
    '@dojo/widgets/variant': {
        extra: 'variant_theme_extra'
    }
}
  1. Add additional classes specified in the classes for both the variant and base css
// theme
{
    '@dojo/widgets/base': {
        root: 'base_theme_root'
    },
    '@dojo/widgets/variant': {
        extra: 'variant_theme_extra'
    }
}

// classes
{
    '@dojo/widgets/base': {
        root: ['base_classes_root']
    },
    '@dojo/widgets/variant': {
        root: ['variant_classes_root'],
        extra: ['variant_classes_extra']
    }
}

This will result in a theme objected constructed that includes the extra root class from @dojo/widgets/variant and the extra root class for @dojo/widgets/base will be applied in the base widget

  1. Supports a known prefix for classes that need to passed as part of the base widgets theme
    '@dojo/widgets/base': {
        root: ['base_classes_root']
    },
    '@dojo/widgets/variant': {
        baseRoot: ['variant_prefix_theme_root']
    }

When passing base as the prefix will use the variant_prefix_theme_root class as the base widget's root class.

When a prefix is specified, only prefixed classes will be composed for the child's theme. It will not pass the class for an active class, even if the active class exists in the child's css.

Note: Changes the API to automatically create the composed theme to be passed to the base widget's theme property by the user. This saves the user have to constructed the theme every time.

Example Usage:

import theme from '../middleware/theme';
import { tsx, create } from '@dojo/framework/core/vdom';
import Button, { ButtonProperties } from '../button/index';
import * as outlinedButtonCss from '../theme/default/outlined-button.m.css';
import * as buttonCss from '../theme/default/button.m.css';

export interface OutlinedButtonProperties extends ButtonProperties {}

const factory = create({ theme }).properties<OutlinedButtonProperties>();

export const OutlinedButton = factory(function OutlinedButton({
	properties,
	children,
	middleware: { theme }
}) {
	const props = properties();

	return (
		<Button
			{...props}
			theme={theme.compose(
				buttonCss,
				outlinedButtonCss,
			)}
		>
			{children()}
		</Button>
	);
});

export default OutlinedButton;

@dojo-site
Copy link

This pull request has been deployed to:

@dojo-site
Copy link

This pull request has been deployed to:

1 similar comment
@dojo-site
Copy link

This pull request has been deployed to:

@dojo-site
Copy link

This pull request has been deployed to:

@tomdye tomdye mentioned this pull request Dec 3, 2019
5 tasks
@dojo-site
Copy link

This pull request has been deployed to:

@dojo-site
Copy link

This pull request has been deployed to:

Copy link
Member

@tomdye tomdye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have worked this through the scenarios i'm aware of and all appears to be working now

@agubler agubler merged commit 428fc47 into dojo:master Dec 3, 2019
ericos pushed a commit to ericos/widgets that referenced this pull request Jan 3, 2020
* Fix the theme middleware to deal with the compose scenarios

* remove accidental css import

* support variant prefixes overriding the original class in base

* Use variable for THEME_KEY

* Correct types

* Reference variable of CSS_THEME not the string literal

* remove composes of widget variants as they will fallback to the default base theme

* Do not require prefixed classes to compose from the child

* put back grid composes

* remove unused var

* Upgrade deps and add empty css declarations

* Fix scenario of adding classes for prefixed class names

* Do not pass matching classes down that do not have a prefix when a prefix is specified
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants