Customizable styles update

[bennettclark]

Pull request checklist

[ ] Include a change request file using $ npm run change

Description of changes

Add style argument to @Customizable decorator

Microsoft Reviewers: Open in CodeFlow

[phkuo]

What's the intention here? I don't think this is correct. The @Customizable is for global state, like the theme, which is generally the same across the entire page. The 'style' is different though, and unique per component.

[mikewheaton]

@phkuo: This is to enable us to distribute a set of Fluent component styles, which would be passed into Customizer and then apply to all child components. The idea is for teams to be able to opt-in to the styles and test them easily, before those styles are the new defaults.

See Peter's experiment branch for a closer look at how we're wanting to approach this.

[Jahnp]

@phkuo @mikewheaton hit it head-on. This is really to allow deeper, scoped customizations beyond what themes can provide, and pairs with <Customizer>'s scopedSettings.

So for example, we might do something like this to round button corners in a scoped region:

const PrimaryButtonStyles = {
  root: {
    borderRadius: 2,
    boxShadow: '0 2px 4px -0.75px rgba(0, 0, 0, 0.1)'
  }
};


const CompoundButtonStyles = {
  root: {
    borderRadius: 2,
    boxShadow: '0 2px 4px -0.75px rgba(0, 0, 0, 0.1)'
  }
};

const DefaultButtonStyles = {
  root: {
    borderRadius: 2,
    boxShadow: '0 2px 4px -0.75px rgba(0, 0, 0, 0.1)'
  }
};


const NewStyles = {
  PrimaryButton: {
    // Target 'styles' enabled by @customizable('PrimaryButton', ['styles'])
    styles: PrimaryButtonStyles
  },
  DefaultButton: {
    styles: DefaultButtonStyles
  },
  CompoundButton: {
    styles: CompoundButtonStyles
  }
}

export class NewStyledComponent extends React.Component<any, any> {
  public render(): JSX.Element {
    return (
      // Components targeted by NewStyles receive style updates in this region
      <Customizer scopedSettings={ NewStyles }>
        <DefaultButton />
      </Customizer>
    )
  }
}

[mikewheaton]

@dzearing or @Jahnp: Can we merge this without getting approval from all of the code reviewers?

[micahgodbolt]

yeah, as long as @dzearing is on board, this doesn't change any of the functionality, so wouldn't affect codeowners

[Jahnp]

@mikewheaton -- Yup, we can do that, just want to make sure there isn't any harm with changing the examples as well. I don't see how that'd be a problem though.

[phkuo]

@mikewheaton @Jahnp It is my understanding that the 'styles' param was meant to be passed in directly to an INSTANCE of a component. Won't adding it to @Customizable cause any specifically-passed-in 'styles' to be overridden by the 'styles' in the current context, or vice versa? To phrase it more generically, should parameters controlled by @Customizable only be passed in by the Customizer framework? How does it interact with params directly passed into the instance of the component?

[dzearing]

This change is the least intrusive, but it's also very repetitive. Would it make sense to default the customizable props to them and styles?

[dzearing]

@phkuo If you pass in custom styling as a prop, your value will stomp on injected styling. It certainly is a weakness. Let's tackle the "deep merge" scenario separately.