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

Styling a Widget Instance #270

Closed
kitsonk opened this issue Aug 11, 2017 · 7 comments
Closed

Styling a Widget Instance #270

kitsonk opened this issue Aug 11, 2017 · 7 comments
Assignees
Milestone

Comments

@kitsonk
Copy link
Member

kitsonk commented Aug 11, 2017

User Story

As a developer of Dojo 2, I need to be able to modify the styling of individual instances.

Specific Use Case

I have created an instance of @dojo/widgets/Button in my application, I am using the base Dojo 2 theme, but I need to change the style of this instance of the button, making the font bold and changing the background colour to #FFCCED.

@kitsonk kitsonk added this to the 2017.08 milestone Aug 11, 2017
@kitsonk kitsonk added the beta3 label Aug 11, 2017
@kitsonk kitsonk modified the milestones: 2017.08, 2017.09 Sep 4, 2017
@smhigley
Copy link
Contributor

The options currently available to change a widget instance's style are:

  • The style object
  • extraClasses
  • Extending Button

I'm almost never in favor of inline styles, especially when mixed with stylesheets. For extraClasses vs. extension, I think it comes down to balancing whether it really is just a one-time one-style-change thing, or whether additional variations will be required. For Button, I can imagine something like the following example becoming a common pattern:

CustomButton.ts

export interface CustomButtonProperties extends ButtonProperties {
    appearance?: 'small' | 'large' | 'confirm' | 'cancel';
}

@theme(css)
export default class CustomButton extends Button<CustomButtonProperties> {
    getRootClasses() {
        const {
            appearance = 'small',
            disabled,
            popup,
            pressed
        } = this.properties;
        return this.classes(
            css.root,
            appearance === 'small' ? css.small : css.large,
            disabled ? css.disabled : null,
            popup ? css.popup : null,
            pressed ? css.pressed : null
        );
    }
}

Usage

w(CustomButton, {
    appearance: 'large'
}, [ 'foo' ]),

This doesn't work with the current Button widget, but it should if we implement some changes to all widgets to allow better extensibility.

@smhigley
Copy link
Contributor

As @agubler pointed out, there's also the possibility of extending Button for each separate "appearance":

@theme(smallButtonCss)
export class SmallButton extends Button { }

@theme(largeButtonCss)
export class LargeButton extends Button { }

@theme(confirmButtonCss)
export class ConfirmButton extends Button { }

@theme(cancelButtonCss)
export class CancelButton extends Button { }

@bitpshr
Copy link
Member

bitpshr commented Oct 20, 2017

Nice concise writeup @smhigley. Now that we have #314 and #316 to help with general component extensibility, the first CustomButton example becomes more realistic. It's good that we'll support both approaches, as I think the first approach is intuitive, but it sounds like we've come to favor the cleanliness of the second approach that leverages the theme decorator for most cases. This approach doesn't force downstream users to override a CSS classname-generating function and to make sure all necessary root classes are still returned, which could be tedious. Still, drawbacks to this approach include CSS organization issues and needing to duplicate styles of overridden classes, neither of which should be taken lightly.

Action items:

@kitsonk
Copy link
Member Author

kitsonk commented Oct 21, 2017

Just reading this now... Thanks, this seems to be a great logical way of doing this! In fact, I am just going to try this pattern now on web-editor.

@kitsonk
Copy link
Member Author

kitsonk commented Oct 21, 2017

I am trying the second solution, as when "binding" to a particular sub-widget I am using in a contained widget:

@theme(smallButtonCss)
export class SmallButton extends Button { }

And while it appears to work, I get the warning in the console of:

Duplicate base theme class key 'root' detected, this could cause unexpected results @Themeable.ts:366 

@agubler
Copy link
Member

agubler commented Oct 21, 2017

@kitsonk there is an issue to remove the warning as it’s no longer desirable following the direction we’ve taken for extending widgets

@kitsonk
Copy link
Member Author

kitsonk commented Oct 21, 2017

Cool! 👍

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

No branches or pull requests

6 participants