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

Creating a new Widget and Styles #273

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

Creating a new Widget and Styles #273

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

Comments

@kitsonk
Copy link
Member

kitsonk commented Aug 11, 2017

User Story

As a developer, when I create a new class of widget, I want it to be easily themeable. What considerations and conventions should I include in the styles for the widget to ensure it is themeable downstream?

@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
@eheasley eheasley added beta4 and removed beta3 labels Oct 3, 2017
@smhigley
Copy link
Contributor

smhigley commented Oct 5, 2017

The most basic way to answer this is just that all VNodes should get passed a className in this.classes() even if they receive no styles in the current theme, and all WNodes should get passed the theme property. This will at minimum give any authors using the widget the ability to theme it any way they wish.

There are a couple other patterns that will make the experience of theming a widget a little nicer: fixed classes, and making it easy to extend the widget and add to state classes (although the second may be more a widget extension pattern than a theming one).

First, fixed classes: any styles that are necessary for the basic function of the widget should be split out into fixed classes so widget authors don’t need to worry about breaking functionality while trying to modify visual style. A good example of this is the Slider widget, which relies on an invisible native slider input overlaid on the styled interface. There are a number of styles necessary to correctly make the native input invisible and correctly positioned, all of which would be a pain to re-create every time an author wished to thumb size, for example.

The second point, isolating and returning “state”-type classes in their own overridable method is something we don’t do yet in any existing widgets. For example: a Button already has classes for disabled, popup, and pressed states. If we returned those classes in a getStateClasses() or getModifierClasses() function, it would be easy to override and add something like a loading class. This ties in to #274, in that I think we need to accept that we will never be able to account for all the visual states a widget may need to be able to handle. In the above case, creating an extended Button that includes a loading state might look like this:

export interface CustomButtonProperties extends ButtonProperties {
  loading: boolean;
}

@theme(css)
export class CustomButton extends Button<CustomButtonProperties> {
  getStateClasses() {
    const { disabled, popup, pressed, loading } = this.properties;
    return this.classes(
      disabled ? css.disabled : null,
      popup ? css.popup : null,
      pressed ? css.pressed : null,
      loading ? css.loading : null
    );
  }
}

Summary

  • All nodes have a class applied, even if not currently used
  • Pass theme to any child widgets
  • Styles necessary for the basic function of the widget are separated into fixed classes
  • [proposed] Widgets that support different visual states return those classes in an overridable function

@eheasley eheasley modified the milestones: 2017.09, 2017.10 Oct 6, 2017
@bitpshr
Copy link
Member

bitpshr commented Oct 20, 2017

@smhigley summarized this pretty perfectly. #316 covers returning “state”-type classes in their own method and our current theme tutorial covers the other three points listed.

Action items:

@bitpshr bitpshr closed this as completed Oct 20, 2017
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

4 participants