-
Notifications
You must be signed in to change notification settings - Fork 53
Adding component specs for Button, Checkbox, Icon, Link, Menu and Slider #2134
Conversation
Generated by 🚫 dangerJS |
…into docs/componentSpecs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should get drafts in here to start a review process and make updates until we are happy with them.
|
||
## Related variant considerations | ||
|
||
- Toggle: should be a separate component because it has different anatomy & warrants unique themability. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GROUP REVIEW:
We should pull the "why" behind stances like this into a Design Principles document. There is a branch started for this. @levithomason to update.
|
||
## Reference implementations | ||
|
||
https://codesandbox.io/s/checkboxes-ggpx1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GROUP REVIEW:
Would be awesome to have a page like this in the repo where devs can do in depth research on how other libraries think about the same component.
specs/Checkbox.md
Outdated
| ---- | ---- | ------------- | | ||
|
||
|
||
### Recommended props |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GROUP REVIEW:
We should note what prop interfaces are being extended as well.
specs/Checkbox.md
Outdated
|
||
| Name | Type | Notes | | ||
| -------------------- | -------------------------------------------------------- | -------------------------------------------------------------------------------| | ||
| animation | AnimationProp | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GROUP REVIEW:
This is solved by agreeing on component best practices. This is a global shorthand prop on all components. It allows <Animation />
to be easily added to any component. The same practice is followed for other components like <Tooltip />
and <Popup />
. The idea is, make it easier to use components, doing things like animating should be easy.
| root | label | | ||
| input | actual checkbox element - what gets checked/unchecked | | ||
| icon | visual checkmark, sideways if indeterminate | | ||
| box | wraps input & icon - what actual gets the styling | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GROUP REVIEW:
What are our design principles for slots? Inline replaceable? Can I add a slot to a component? Etc.
|
||
Both Fluent and Teams themes and other custom themes will be made with compose and the design tokens specified below. Screenshots of themed variants will be posted here soon after that work is done like the example code below. | ||
|
||
The `Checkbox` uses `react-texture` to provide a recomposable implementation that has no runtime performance penalties. The `BaseCheckbox` implementation can be used to provide new `slots` and default `props`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GROUP REVIEW:
Reminder, react-texture to be replaced here.
…ist, list of prop differences
This PR brings the component specs that have been reviewed internally by the Fabric team and reside in our staging repo into the main repo so that we can have a larger conversation about these components.
Microsoft Reviewers: Open in CodeFlow