Skip to content

Conversation

@kysedate
Copy link
Contributor

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ npm run change

Description of changes

Adding a prop to the Spin Button that allows for a complete override of the fabric styles

@msftclas
Copy link

msftclas commented Jan 20, 2018

CLA assistant check
All committers have signed the CLA.

@kysedate kysedate changed the title Kysedate/spin button get class names prop addition SpinButton: Add getClassNAmes prop to allow complete customization of the component Jan 20, 2018
@kysedate kysedate changed the title SpinButton: Add getClassNAmes prop to allow complete customization of the component SpinButton: Add getClassNames prop to allow complete customization of the component Jan 20, 2018
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "SpinButton: Add getClassNAmes prop to allow complete customization of the component",
Copy link
Member

Choose a reason for hiding this comment

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

rather nervous of this change. A better approach would be to avoid the getClassNames approach and use the 6.0 getStyles functional approach. We found that getClassNames required us to come up with every possible permutation of areas and states, whereas a function of state can produce a given set of area classes.

See the ghdocs/styling.md file for more description.

Copy link
Member

Choose a reason for hiding this comment

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

@dzearing this is another case where we need something that lets us completely override the styling in the 5.0 timeframe for Office Online.

Copy link
Member

Choose a reason for hiding this comment

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

Should we just move it to getStyles in the 5.0 branch?

@christiango
Copy link
Member

@montselozanod can you work with @kysedate to see how we can unblock this work? Can we perhaps go straight to the new styling API safely?

@montselozanod
Copy link
Member

@christiango sure!! I'll talk with @kysedate so we can go over this together

@jspurlin jspurlin merged commit 0fdbf1d into microsoft:master Feb 13, 2018
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants