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

Theming/Styling contained Widgets #274

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

Theming/Styling contained Widgets #274

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 creating a contained themed widget, I want to be able to influence the theme and styling of the contained widgets.

Specific Use Case

I have created a re-usable panel, which has a border and contains a button and a slider. Based on the configuration of the panel, I want to change the background colour of the button and the thumbnail colour of the the slider.

@kitsonk kitsonk added this to the 2017.08 milestone 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

It seems like the way we’re moving towards for pretty much any style change for an existing widget is extension with a new @theme directive. I don’t believe framing it from the view of creating a container widget is all that relevant, since it should be up to the component widgets to support every possible visual style throughout the app. I can think of two general approaches: either create a separate widget for each visual style, or create a single extended widget that supports multiple visual styles, toggled by a property.

Solution 1, multiple widgets:

Buttons:

@theme(css)
export class RedButton extends Button { }

@theme(css)
export class BlueButton extends Button { }

Usage in Container:

render() {
  const { buttonType } = this.properties;
  return v(‘div’, [
    w(buttonType === ‘red’ ? RedButton : BlueButton, [  Button Text  ])
  ]);
}

Solution 2, property controlling appearance:

Button:

export interface MyButtonProperties extends ButtonProperties {
  appearance: ‘default’ | ‘alternate’;
}

@theme(css)
export class MyButton extends Button<MyButtonProperties> {
  getModifierClasses() {
    const { disabled, appearance } = this.properties;
    return this.classes(
      disabled ? css.disabled : null,
      appearance === ‘default’ ? css.red : css.blue
    );
  }
}

Usage in Container:

render() {
  const { buttonType } = this.properties;
  return v(‘div’, [
    w(MyButton, {
      appearance: buttonType
    }, [ ‘Button Text’ ])
  ]);
}

The first solution looks cleaner at first, but I’m worried that a larger number of variations will result in an explosion of different widgets and CSS module files. There’s also the problem that simple variations like red vs. blue are easy, but what about cases where you might have two different types of variation, e.g. color and size. With the first solution you would need RedButton, LargeRedButton, BlueButton, LargeBlueButton, etc., whereas for the second you would only need to add buttonType and size properties.

There’s also the CSS organization issue. If we approach this by suggesting separate modules for each slight variation in styling, each module will need to duplicate multiple styles like font-face, line-height, border, etc. just to change background-color on the root class. This seems like it would become brittle and difficult to maintain. Adding additional classes to the root node controlled through properties would more closely mirror the way I prefer to organize styles even without modules, i.e. <button class=“button large callout”> as opposed to <button class=“largeCalloutButton”>.

We could somewhat mitigate the duplication issue with composes, but I worry the over-use of composes will make it difficult to sort through which styles come from where. I would personally prefer to explicitly have all my button styles in a single css module file than try to sort through five to find the border-width declaration.

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

bitpshr commented Oct 20, 2017

This is directly related to #270, since as @smhigley pointed out, it's not extremely relevant if a component is contained or not. This boils down to styling a component instance based on properties, which lends itself perfectly to an extended component using an extended properties interface as captured by #314.

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