From db9d2b0b4c500023f19da128f43cf061930f105a Mon Sep 17 00:00:00 2001 From: KHMakoto Date: Wed, 27 Nov 2019 16:26:27 -0800 Subject: [PATCH 01/11] Adding component specs for Checkbox, Icon, Link and Slider. --- specs/Checkbox.md | 388 +++++++++++++++++++++++++++++++++++ specs/Icon.md | 506 ++++++++++++++++++++++++++++++++++++++++++++++ specs/Link.md | 419 ++++++++++++++++++++++++++++++++++++++ specs/Slider.md | 442 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 1755 insertions(+) create mode 100644 specs/Checkbox.md create mode 100644 specs/Icon.md create mode 100644 specs/Link.md create mode 100644 specs/Slider.md diff --git a/specs/Checkbox.md b/specs/Checkbox.md new file mode 100644 index 0000000000..245edeea28 --- /dev/null +++ b/specs/Checkbox.md @@ -0,0 +1,388 @@ +# Checkbox component specification + +The `Checkbox` component allows a user to choose between two mutually exclusive options. + +## Related variant considerations + +- Toggle: should be a separate component because it has different anatomy & warrants unique themability. + +- Indeterminate/Tri State: should support groups with hierarchical checkboxes where the parent checkbox can be in a mixed state if its children checkboxes aren't all checked (and if they are all checked, then the parent is checked; same if they are all unchecked, then the parent is unchecked). + +## Reference implementations + +https://codesandbox.io/s/checkboxes-ggpx1 + +Note about the Stardust example: there's some weirdness with how the theme providers are interacting with each other, the Stardust checkbox's styling is messing up as a result. + +Fabric Checkbox [docs](https://developer.microsoft.com/en-us/fabric#/controls/web/Checkbox) + +Stardust Checkbox [docs](https://microsoft.github.io/fluent-ui-react/components/checkbox/definition) + +Material UI Checkbox [docs](https://material-ui.com/components/checkboxes/) + +BaseUI Checkbox [docs](https://baseweb.design/components/Checkbox/) + +Chakra Checkbox [docs](https://chakra-ui.com/Checkbox) + +Cabon Checkbox [docs](https://www.carbondesignsystem.com/components/checkbox/code) + +AntD Checkbox [docs](https://ant.design/components/Checkbox/) + +FastDNA Checkbox [docs](https://explore.fast.design/components/Checkbox) + + +## Props + +> TODO: Consult the prop wizard to derive consistently defined props. + +| Name | Type | Default value | +| ---- | ---- | ------------- | + + +### Recommended props + +| Name | Type | +| -------------- | ----------------------------------------------------------- | +| ariaDescribedBy | string | +| ariaLabel | string | +| ariaLabelledBy | string | +| as | React.ElementType | +| checked | boolean | +| className | string | +| defaultChecked | boolean | +| defaultIndeterminate | boolean | +| disabled | boolean | +| indeterminate | boolean | +| label | string | +| name | string | +| onChange | (ev: Event, value: boolean) => void | +| vertical | boolean | + +Note: rtl, styles, and theme come from compose or the ThemeProvider. And name has been added to support checkbox in form scenarios. + +Removing the following two props because the ARIA spec dictates role='checkbox' doesn't need aria-posinset and aria-setsize. These are only valid for role='option' which is only in the case the checkbox is a part of a listbox, which is not something we need to account for in the base component API. If the user does need to provide these two props, slotProps could be used to apply additional props to any slot. + +| Name | Concern | +| ------------------------------------- | ----------------------------------------------------------------- | +| ariaPositionInset | if checkbox is in a set, should be up to the user to provide a11y | +| ariaSetSize | same as above | + +### Fabric Checkbox props + +https://developer.microsoft.com/en-us/fabric#/controls/web/checkbox + + +| Name | Type | Notes | +| -------------------- | -------------------------------------------------------- | -------------------------------------------------------------------------------| +| ariaLabel | string | | +| ariaDescribedBy | string | | +| ariaLabelledBy | string | | +| ariaPositionInset | number | | +| ariaSetSize | number | | +| boxSide | 'start' or 'end' | default 'start' | +| checked | boolean | | +| checkmarkIconProps | IIconProps | | +| className | string | | +| componentRef | IRefObject | | +| defaultChecked | boolean | | +| defaultIndeterminate | boolean | | +| disabled | boolean | | +| indeterminate | boolean | | +| inputProps | React.ButtonHTMLAttributes| | +| keytipProps | IKpeytipProps | | +| label | string | | +| onChange | (ev, checked) => void | | +| onRenderLabel | IRenderFunction | | +| styles | IStyleFunctionOrObject| | +| theme | ITheme | | + +### Stardust Checkbox props + + +| Name | Type | Notes | +| -------------------- | -------------------------------------------------------- | -------------------------------------------------------------------------------| +| animation | AnimationProp | | +| as | React.ElementType | default type is "div" | +| className | string | | +| content | ReactNode | | +| design | ComponentDesign | | +| disableAnimations | boolean | default false | +| overwrite | boolean | default false | +| renderer | Renderer | | +| rtl | boolean | default false | +| styles | ComponentSlotStyle | | +| target | Document | | +| theme | ThemeInput | | +| variables | any | | + +### Differences of Fabric/Stardust to resolve + +| Name | Type | Notes | +| -------------------- | -------------------------------------------------------- | -------------------------------------------------------------------------------| +| animation | AnimationProp | | +| disableAnimations | boolean | default false | +| overwrite | boolean | default false | +| renderer | Renderer | | +| variables | any | | +| target | Document | | +| content | ReactNode | | +| ariaPositionInSet | number | if checkbox is in a set, should be up to the user to provide a11y | +| ariaSetSize | number | | + +### Conversion process from Fabric 7 to Fluent UI Checkbox + +Props being changed: + +Some props, like style & className, should always go into the root or to the applicable element like name into the box element (replacing input). +Data-attributes will be spread using slotProps. + +Props being removed: + +ariaPoisitionInSet and ariaSetSize - when writing parent component, user should set these on the checkbox. +animations: remnant pattern of semantic UI, don't need animations for checkbox theming +defaultChecked: overloading with checked - can just set default value of checked. + +## Slots + +| Name | Considerations | +| --------- | ------------------------------------------------------ | +| 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 | + +## DOM structure + +General considerations: + +Only use as toggle between two mutually exclusive options (binary) or in a group with shared context to offer multiple options. + +Uncontrolled vs. controlled: implemented in the prototype already through useControlledState React hook. +Indeterminate state: When children checkboxes aren't checked, don't check parent checkbox. + +Could consider supporting an invalid state/error state but this might just be supported via styling that's passed in by the user and done through compose. + +### Recommended DOM + +```html + +``` + +### Fabric Checkbox example DOM + +```html +
+ + +
+``` + +### Stardust Checkbox example DOM + +```html + +``` +### MUI Checkbox example DOM + +```html + +``` + +### Behaviors + +Aria spec: https://www.w3.org/TR/wai-aria-practices-1.1/#checkbox +https://www.w3.org/TR/wai-aria-practices/#checkbox + +Fluent UI HIG: https://microsoft.sharepoint-df.com/:w:/t/OPGUXLeads/EbBiGJ-gLPFGszdhSxb8X5IBFik0ax7wZLJc8FlDXOwDYA?e=Cy4Er3 + +### Disabled state + +Use `aria-disabled`. Screenreaders should let users know of the existence of the checkbox but it should be read-only. Ignore all events & no change to `checked` value allowed. + +### Checked state + +`aria-checked` indicates whether element is checked (`true`) or unchecked (`false`) but can also be `mixed` which represents a tri-state (indeterminate) input in a situation with a group of other elements that have a mixture of checked and unchecked values. + +### Indeterminate state + +Mixed state checkbox represents a checkbox that can support a partially checked state. If none of the checkboxes in a set are checked, the mixed state checkbox isn't checked (and if all are checked then so is the mixed state) but if the set contains a mix of checked and unchecked boxes, then the tri-state is appropriate so `aria-checked` will be set to `mixed`. Here's an example: https://www.w3.org/TR/wai-aria-practices/examples/checkbox/checkbox-2/checkbox-2.html + + +### Focus indicators + +Focus indicators should not show in mouse or touch interaction; they should only appear when keyboard tabbing/directional keystrokes are pressed, and should disappear when mouse/touch interactions occur. + +### Keyboarding +| Key | Description | +| --------- | -------------------------------------------------------------------- | +| Tab | Moves keyboard focus to the checkbox. | +| Space | Toggles checkbox between checked and unchecked states. | + +### Mouse input + +- `mouseenter` should change the styling of checkbox to hovered state (preview what it looks like to be toggled but not full styling - checkmark, but not background color for example as in current Fabric 7 checkbox). +- `mouseleave` should change the styling of checkbox back to non-hovered state (so remove the preview of checked state) +- `mousedown` toggle state +- `mouseup` apply styling of new state + +### Touch + +Same behavior as above except no preview of toggled state through hover. + +### Screenreader accessibility: + +#### `root`: + +- should render the native element using the `as` prop, defaulting to `div` +- should mix in native props expected for the element type defined in `as`. + +Input slot: role should be set to `checkbox` +A visible label referenced by the value of `aria-labelledby` (id of element containing the label) set on the element with role `checkbox`. +If there's additional static text representing that is descriptive, `aria-describedby` should be set to id of element containing the description. +`aria-label` set on the element with role `checkbox`. + +### Accessibility concerns for the user + +`aria-label`, `aria-labelledby`: Describe what is the purpose of the checkbox, latter points to id of element with former. + +### Themability and customization + +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`: + +```tsx +const FooCheckbox = BaseCheckbox.compose({ + tokens: {}, + styles: {}, + slots: {} +}); + +render() { + + This renders as a checkbox + +} +``` + +### Composition + +1 per slot +1 per state, tagged on root + +### Component design tokens + +> Tokens represent the general look and feel of the various visual slots. Tokens feed into the styling at the right times in the right slot. +> +> Regarding naming conventions, use a camelCased name following this format: +> `{slot}{property}{state (or none for default)}`. For example: `labelSizeHovered`. +> +> Common property names: `size`, `background`, `color`, `borderRadius` +> +> Common states: `hovered`, `pressed`, `focused`, `checked`, `checkedHovered`, `disabled` + +| Name | Considerations | +| ------------------ | -------------- | +| boxBorderColor | | +| boxBorderRadius | | +| boxBorderWidth | | +| boxColor | | +| boxColorDisabled | | +| boxColorFocused | | +| boxColorHovered | | +| boxColorPressed | | +| boxSize | | +| labelColor | | +| labelColorDisabled | | +| labelColorFocused | | +| labelColorHovered | | +| labelColorPressed | | +| labelSize | | +| iconColor | | +| iconColorDisabled | | +| iconColorFocused | | +| iconColorHovered | | +| iconColorPressed | | +| iconSize | | + +NOTE! Stardust does not follow this convention. Their Checkbox currently uses these tokens: + +``` +background: string +disabledBackground: string +disabledBackgroundChecked: string +toggleBackground: string +toggleBorderColor: string +toggleIndicatorColor: string +toggleIndicatorSize: string +checkedBackground: string +checkedBorderColor: string +checkedBackgroundHover: string +checkedIndicatorColor: string +checkboxCheckedColor: string +checkboxToggleCheckedBackground: string +disabledToggleBackground: string +gap: string +borderColor: string +borderColorHover: string +checkboxColor: string +checkboxToggleCheckedBorderColor: string +checkedTextColor: string +disabledColor: string +disabledBorderColor: string +disabledToggleBorderColor: string +disabledCheckboxColor: string +disabledToggleIndicatorColor: string +disabledCheckedIndicatorColor: string +textColor: string +textColorHover: string +indicatorColor: string +``` +## Considerations for different screen sizes + +Won't really look or behave differently in context of different phone/tablet/desktop sizes - as in different sizes would not cause this component to look or behave differently. + +## Use cases + +The `Checkbox` component may be used within a `Form` component by providing the name prop to indicate the name of the input element to be fed into the form action. Example: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/checkbox + +## Compatibility with other libraries + +> TODO: If this component represents a selected value, how will that be used in an HTML form? Is there a code example to illustrate? + +> TODO: Is it possible this component could be rendered in a focus zone? If so, should the focus model change in that case? \ No newline at end of file diff --git a/specs/Icon.md b/specs/Icon.md new file mode 100644 index 0000000000..89a13111f7 --- /dev/null +++ b/specs/Icon.md @@ -0,0 +1,506 @@ +# Icon component specification + +The `Icon` component provides a symbol, graphic or image that represents an application, a capability, or some other concept or specific entity with meaning for the user. + +## Related variant considerations + +The following section documents variants of the component that currently exist in Fabric and identifies variants that exist in other component libraries but don't currently exist in Fabric, documenting which component libraries have those variants. + +### Variants existing in Fabric today + +- `Font icons` +- `Image icons` + +### Variants not in Fabric but that exist in other component libraries + +- `Block icons` + - In Gestalt +- `Bordered icons` + - In Semantic UI + - In Stardust +- `Circular icons` + - In Semantic UI + - In Stardust +- `Corner icons` + - In Semantic UI +- `Focusable icons` + - In Chakra UI +- `Group icons` + - In Semantic UI +- `Inverted icons` + - In Semantic UI +- `Outlined vs filled icons via props` + - In Ant Design +- `Rotated icons` + - In Ant Design + - In Semantic UI + - In Stardust +- `Spinning/Loading icons` + - In Ant Design + - In Semantic UI +- `Two tone icons` + - In Ant Design + - In Atlaskit + +## Reference implementations + +The following section documents links to different UI libraries implementations of Icons, while also providing a code sandbox with a side by side implementation of them for comparison. + +- [Side-by-side implementations](https://codesandbox.io/s/icon-implementations-4d5gp) + +- [Ant Design Icon docs](https://ant.design/components/icon/) + +- [Atlaskit Icon docs](https://atlaskit.atlassian.com/packages/core/icon) + +- [Base Web Icon docs](https://baseweb.design/components/icon/) + +- [Chakra UI Icon docs](https://chakra-ui.com/icon) + +- [Elemental UI Icon docs](http://elemental-ui.com/forms) + - Look for the `Icons` section in the `Forms` page + +- [Fabric Icon docs](https://developer.microsoft.com/en-us/fabric#/controls/web/icon) + +- [Gestalt Icon docs](https://pinterest.github.io/gestalt/?ref=designrevision.com#/Icon) + +- [Material-UI Icon docs](https://material-ui.com/components/icons/) + +- [Semantic UI Icon docs](https://react.semantic-ui.com/elements/icon/) + +- [Stardust Icon docs](https://microsoft.github.io/fluent-ui-react/components/icon/definition) + +## Props + +The following section documents the properties that will become part of the new component, as well as the process for mitigating all changes when moving from Fabric and Stardust to Fluent UI. + +> TODO: Consult the prop wizard to derive consistently defined props. + +| Name | Type | Default value | Description | +| ---- | ---- | ------------- | ----------- | + +### Recommended component props + +| Name | Type | Default value | Description | +| ---------------- | :-------: | :-----------: | ------------------------------------------------------------------------------------------------------------------------------------------- | +| `ariaHidden` | `boolean` | `false` | Indicates whether the element is exposed to an accessibility API. | +| `ariaLabel` | `string` | | Defines a string value that labels the current element. | +| `ariaLabelledby` | `string` | | Identifies the element (or elements) that labels the current element. | +| `as` | `string` | | Defines a component that should be used as the root element of the `Icon`. | +| `className` | `string` | | Defines an additional classname to provide on the root of the `Icon`. | +| `name` | `string` | | Defines the name of the pre-registered `Icon` to use. If the string is empty, a placeholder blank space of the same width will be rendered. | +| `role` | `string` | | Defines the accessibility role of the `Icon`. | +| `title` | `string` | | Specifies extra information about the `Icon`. | + +### Props to be discussed + +None at the moment. + +### Fabric Icon props + +https://developer.microsoft.com/en-us/fabric#/controls/web/icon + +#### IIconProps interface + +| Name | Type | Notes | +| -------------- | :----------------------------------------------------: | -------------------------------------------------------------------------------- | +| `ariaLabel` | `string` | | +| `iconName` | `string` | Should we rename this to just be `name`? | +| `iconType` | `IconType` | Already deprecated. | +| `imageErrorAs` | `React.ComponentType` | Should be removed duo to unnecessary complexity and infrequent use. | +| `imageProps` | `IImageProps` | Should not be part of base `Icon`. Should be considered for `ImageIcon` variant. | +| `styles` | `IStyleFunctionOrObject` | Should be deprecated in favor of recomposition. | +| `theme` | `ITheme` | Should not show up in the public props contract. | + +### Stardust Icon props + +#### IconProps interface + +| Name | Type | Notes | +| --------------- | :-------------: | --------------------------------------------------------------------------------------------------------- | +| `accessibility` | `Accessibility` | Why would a user need this prop? | +| `bordered` | `boolean` | Should this prop be provided or is this just a matter that could be solved via styling and recomposition? | +| `circular` | `boolean` | Should this prop be provided or is this just a matter that could be solved via styling and recomposition? | +| `disabled` | `boolean` | Does it make sense for an `Icon` to be `disabled` if it is not an interactive component. | +| `name` | `string` | | +| `outline` | `boolean` | Should this prop be provided or is this just a matter that could be solved via styling and recomposition? | +| `rotate` | `number` | Should this prop be provided or is this just a matter that could be solved via styling and recomposition? | +| `size` | `SizeValue` | Should this prop be provided or is this just a matter that could be solved via styling and recomposition? | +| `xSpacing` | `IconXSpacing` | Should this prop be provided or is this just a matter that could be solved via styling and recomposition? | + +### Conversion process from Fabric 7 to Fluent UI Icon + +#### IIconProps interface + +| Name | Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| -------------- | ------------------------------------- | :--------------------: | :-------------------------------------: | :-------------------: | +| `ariaLabel` | TBD | ❌ | ❌ | ❌ | +| `iconName` | TBD | ❌ | ❌ | ❌ | +| `iconType` | Removing as it is already deprecated. | ☑ | No, because prop is already deprecated. | ❌ | +| `imageErrorAs` | TBD | ❌ | ❌ | ❌ | +| `imageProps` | TBD | ❌ | ❌ | ❌ | +| `styles` | TBD | ❌ | ❌ | ❌ | +| `theme` | TBD | ❌ | ❌ | ❌ | + +### Conversion process from Stardust to Fluent UI Icon + +#### IconProps interface + +| Name | Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| --------------- | ------------------------------------- | :--------------------: | :--------------: | :-------------------: | +| `accessibility` | TBD | ❌ | ❌ | ❌ | +| `bordered` | TBD | ❌ | ❌ | ❌ | +| `circular` | TBD | ❌ | ❌ | ❌ | +| `disabled` | TBD | ❌ | ❌ | ❌ | +| `name` | TBD | ❌ | ❌ | ❌ | +| `outline` | TBD | ❌ | ❌ | ❌ | +| `rotate` | TBD | ❌ | ❌ | ❌ | +| `size` | TBD | ❌ | ❌ | ❌ | +| `xSpacing` | TBD | ❌ | ❌ | ❌ | + +## DOM Structure + +The following section documents the DOM structure for the component from different component library examples and then suggests a recommended DOM taking into consideration common patterns between the libraries reviewed. + +### Ant Design Icon + +#### Example DOM + +```html + + + +``` + +#### Considerations + +- Only supports SVG icons. + +### Atlaskit Icon + +#### Example DOM + +```html + + + + + + +``` + +#### Considerations + +- Only supports SVG icons. +- To use provided icons you need to import built-in icon directly (i.e. `BookIcon`). + +### Base Web Icon + +#### Example DOM + +```html + + + Arrow Up + + + + +``` + +#### Considerations + +- Only supports SVG icons. +- To use provided icons you need to import built-in icon directly (i.e. `ArrowUp`). + +### Chakra UI Icon + +#### Example DOM + +```html + + + + +``` + +#### Considerations + +- Only supports SVG icons. +- Icons are and can be added as part of the theme. + +### Fabric Icon + +#### Example DOM + +##### Font Icon + +```html + +``` + +##### SVG Icon + +```html + +``` + +##### Image Icon + +```html + +``` + +#### Considerations + +- Requires icon initialization and uses global registration. + +### Gestalt Icon + +#### Example DOM + +```html + + + + +``` + +#### Considerations + +- Only supports SVG icons. +- Renders by default as a block, and not an inline, element. + +### Material-UI Icon + +#### Example DOM + +##### Font Icon + +```html + +``` + +##### SVG Icon + +```html + +``` + +#### Considerations + +None. + +### Semantic UI Icon + +#### Example DOM + +```html + +``` + +#### Considerations + +- Only supports font icons. +- Can support icon groups and icon superpositions with the second item in the corner. + +### Stardust Icon + +#### Example DOM + +```html + +``` + +#### Considerations + +- Allows for both font and SVG icons but only has SVG icons built-in. + +### Recommended DOM + +After looking at all the component libraries above and taking into consideration common patterns the following DOM is recommended. + + +```html + +``` + +> TODO: Discuss need to shim back to Fabric with `as=i` because of different tag being used in order to not break styling. + +### Slots + +From the recommended DOM above we can indicate which slots are going to be required: + +| Name | Considerations | +| ----------- | -------------- | +| `root` | | + +> TODO: I really think we should have `ImageIcon` as a separate component, maybe via recomposition, to have a very simple and fast base `Icon`. +> TODO: Do we want a specific `SvgIcon` apart from the `ImageIcon` variant? This seems like one of the most used examples. + +## Behaviors + +Aria spec: +There's no aria spec available for icons. + +Fluent UI HIG: +https://microsoft.sharepoint-df.com/:w:/r/teams/OPGUXLeads/_layouts/15/Doc.aspx?sourcedoc=%7B5113018C-05E7-44BF-B6D4-B164755B8D71%7D&file=Icon.docx&action=default&mobileredirect=true + +### States + +The following section describes the different states in which a `Icon` can be throughout the course of interaction with it. + +#### Default state + +An `Icon` has only one state, its default state. The `Icon` is not an interactive component and it's used only for representational purposes. + +#### States that need discussion + +None. + +### Keyboard interaction + +There is no keyboard interaction that occurs with the `Icon`. + +### Cursor interaction + +There is no cursor interaction that occurs with the `Icon`. + +### Touch interaction + +There is no touch interaction that occurs with the `Icon`. + +### Screen reader accessibility + +#### `root`: + +- Should not be tabbable nor focusable and should have `aria-hidden` applied to it by default. + +#### Accessibility concerns for the user. + +All accessibility concerns would come from user manipulation of the component, so they would be a concern solely for the user and not for the component creator. + +## Themability and customization + +### Composition + +The `Icon` component uses `react-texture` to provide a recomposable implementation that has no runtime performance penalties. The `BaseIcon` implementation can be used to provide new `slots` and default `props` without the application of additional styling: + +```tsx +const FooIcon = BaseIcon.compose({ + tokens: {}, + styles: {}, + slots: {} +}); + +render () { + +} +``` + +### Icon Registration + +#### Fabric + +Fabric uses global registration for its icons which needs a call to an initialization function to be used. Below are wiki and code references into this process: +- [Wiki page](https://github.com/OfficeDev/office-ui-fabric-react/wiki/Using-icons) +- [Icon font generation tool](https://uifabricicons.azurewebsites.net) +- [Main `initializeIcons` function](https://github.com/OfficeDev/office-ui-fabric-react/blob/master/packages/icons/src/index.ts) +- [Icon registration utilities](https://github.com/OfficeDev/office-ui-fabric-react/blob/master/packages/styling/src/utilities/icons.ts) +- [Icon component](https://github.com/OfficeDev/office-ui-fabric-react/tree/master/packages/office-ui-fabric-react/src/components/Icon) + +#### Stardust + +Stardust registers icons in the theme, with all default icons being SVGs but also supporting font icons via: + +```ts +type ObjectOrFunc = ((arg: TArg) => TResult) | TResult; + +type FontIconSpec = ObjectOrFunc<{ + content: string + fontFamily: string +}>; +``` + +Below are wiki and code references into this process: +- [SVG icon processing](https://github.com/microsoft/fluent-ui-react/blob/21c2f9e3e495b3094e0db4610e9f8834cdc135b0/packages/react/src/themes/teams/components/Icon/svg/ProcessedIcons/stardust-icons.sh#L36) +- [Instructions on adding new SVG Icon](https://github.com/microsoft/fluent-ui-react/pull/585) +- [Font icon registration into the theme (fontAwesome theme example)](https://github.com/microsoft/fluent-ui-react/blob/feat/generate-css/src/themes/teams/components/Icon/fontAwesomeIconStyles.ts) +- [Font vs SVG icon rendering](https://github.com/microsoft/fluent-ui-react/blob/master/packages/react/src/themes/teams/components/Icon/iconStyles.ts) +- [Icon styles as part of theme component styles](https://github.com/microsoft/fluent-ui-react/blob/de10e334fc039370c4fe4b425050327d57f3f515/packages/react/src/themes/teams/componentStyles.ts#L51) +- [Merging icons as part of theme](https://github.com/microsoft/fluent-ui-react/blob/feat/generate-css/src/themes/teams/index.ts) + +> - TODO: Decide on recommended thing to do. Leaning towards Stardust approach but worried about perf implications regarding icon definitions. +> - TODO: Discuss how to handle font loading if we put icons in the theme. +> - Should font loading also be part of the theme? +> - Has to be loaded somehow and is ok for the majority of customers to automatically load them, but some customers need to prevent this loading from MSFT CDNs. + +## Class names + +1 per slot +1 per state, tagged on root + +### Component design tokens + +> Tokens represent the general look and feel of the various visual slots. Tokens feed into the styling at the right times in the right slot. +> +> Regarding naming conventions, use a camelCased name following this format: +> `{slot}{property}{state (or none for default)}`. For example: `thumbSizeHovered`. +> +> Common property names: `size`, `background`, `color`, `borderRadius` +> +> Common states: `hovered`, `pressed`, `focused`, `checked`, `checkedHovered`, `disabled` + +| Name | Considerations | +| ------------------------- | -------------- | +| `color` | | +| `fontSize` | | +| `fontWeight` | | + +## Use cases + +> TODO: Example use cases + +## Compatibility with other libraries + +> TODO: If this component represents a selected value, how will that be used in an HTML form? Is there a code example to illustrate? + +> TODO: Is it possible this component could be rendered in a focus zone? If so, should the focus model change in that case? diff --git a/specs/Link.md b/specs/Link.md new file mode 100644 index 0000000000..b029cc4585 --- /dev/null +++ b/specs/Link.md @@ -0,0 +1,419 @@ +# Link component specification + +The `Link` component is a clickable control primarily used for navigation, providing an interactive reference to a resource. It is usually displayed as an inline element by default that can wrap text if it goes past the edges of its parent. + +## Related variant considerations + +The following section documents variants of the component that currently exist in Fabric and identifies variants that exist in other component libraries but don't currently exist in Fabric, documenting which component libraries have those variants. + +### Variants existing in Fabric today + +- `Link rendered as an anchor` +- `Link rendered as a button` + - Removing this variant by default from Fluent UI, if people want it they can use the `slots` or the `as` prop. + +### Variants not in Fabric but that exist in other component libraries + +- `Block/Non-inline link` + - In Carbon Design + - In Gestalt +- `External link` + - In Chakra UI + +## Reference implementations + +The following section documents links to different UI libraries implementations of Links, while also providing a code sandbox with a side by side implementation of them for comparison. + +- [Side-by-side implementations](https://codesandbox.io/s/link-implementations-utdpb) + +- [Base Web Link docs](https://baseweb.design/components/link/) + +- [Carbon Design Link docs](https://www.carbondesignsystem.com/components/link/code) + +- [Chakra UI Link docs](https://chakra-ui.com/link) + +- [Fabric Link docs](https://developer.microsoft.com/en-us/fabric#/controls/web/link) + +- FastDNA Link (Hypertext) + - [Docs](https://github.com/microsoft/fast-dna/tree/master/packages/fast-components-react-base/src/hypertext) + - [Example](https://explore.fast.design/components/hypertext) + +- [Gestalt Link docs](https://pinterest.github.io/gestalt/?ref=designrevision.com#/Link) + +- [Material-UI Link docs](https://material-ui.com/components/links/) + +## Props + +The following section documents the properties that will become part of the new component, as well as the process for mitigating all changes when moving from Fabric and Stardust to Fluent UI. + +### Recommended component props + +| Name | Type | Default value | Required? | Description | +| ----------------- | :------------------------: | :-----------: | :-------: | -------------------------------------------------------------------------------------- | +| `ariaDescribedBy` | `string` | | No | Identifies the element (or elements) that describes the object. | +| `ariaHidden` | `boolean` | `false` | No | Indicates whether the element is exposed to an accessibility API. | +| `ariaLabel` | `string` | | No | Defines a string value that labels the current element. | +| `ariaLabelledBy` | `string` | | No | Identifies the element (or elements) that labels the current element. | +| `className` | `string` | | No | Defines an additional classname to provide on the root of the `Link`. | +| `componentRef` | `IRefObject` | | No | Defines an optional reference to access the imperative interface of the `Link`. | +| `disabled` | `boolean` | `false` | No | Defines whether the `Link` is in an enabled or disabled state. | +| `href` | `string` | | Yes | Defines an href that serves as the navigation destination when clicking on the `Link`. | +| `onClick` | `(ev: MouseEvent) => void` | | No | Defines a callback that handles the processing of click events on the `Link`. | +| `role` | `string` | | No | Defines the accessibility role of the `Link`. | + +Props no outlined above are not handled and should be spread in the `root` slot of the component. + +### Recommended interface props + +| Name | Type | Default value | Description | +| ------- | :----------: | ------------- | ------------------------- | +| `focus` | `() => void` | | Sets focus on the `Link`. | + +### Props to be discussed + +None at the moment. + +### Fabric Link props + +https://developer.microsoft.com/en-us/fabric#/controls/web/link + +#### ILink interface + +| Name | Type | Notes | +| ------- | :----------: | ----- | +| `focus` | `() => void` | | + +#### ILinkProps interface + +| Name | Type | Notes | +| -------------- | :----------------------------------------------------------: | ------------------------------------------------------ | +| `as` | `string \| React.ComponentClass \| React.StatelessComponent` | Remove `as` prop in new component. | +| `componentRef` | `IRefObject` | | +| `disabled` | `boolean` | | +| `keytipProps` | `IKeytipProps` | Should be removed until we add `Keytips` in Fluent UI. | +| `styles` | `IStyleFunctionOrObject` | Should be deprecated in favor of recomposition. | +| `theme` | `ITheme` | Should not show up in the public props contract. | + +### Stardust Link props + +Stardust does not currently have a `Link` component implementation. + +### Conversion process from Fabric 7 to Fluent UI Link + +#### ILink interface + +| Name | Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| ------- | -------------------- | :--------------------: | :--------------: | :-------------------: | +| `focus` | TBD | ❌ | ❌ | ❌ | + +#### ILinkProps interface + +| Name | Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| -------------- | -------------------- | :--------------------: | :--------------: | :-------------------: | +| `as` | TBD | ❌ | ❌ | ❌ | +| `componentRef` | TBD | ❌ | ❌ | ❌ | +| `disabled` | TBD | ❌ | ❌ | ❌ | +| `keytipProps` | TBD | ❌ | ❌ | ❌ | +| `styles` | TBD | ❌ | ❌ | ❌ | +| `theme` | TBD | ❌ | ❌ | ❌ | + +### Conversion process from Stardust to Fluent UI Link + +Stardust does not currently have a `Link` component implementation. + +## DOM Structure + +The following section documents the DOM structure for the component from different component library examples and then suggests a recommended DOM taking into consideration common patterns between the libraries reviewed. + +### Base Web Link + +#### Example DOM + +```html + + Link to Base Web + +``` + +#### Considerations + +None. + +### Carbon Design Link + +#### Example DOM + +```html + + Link + +``` + +#### Considerations + +- Renders by default as a block, and not an inline, element. + +### Chakra UI Link + +#### Example DOM + +```html + + Chakra UI + +``` + +#### Considerations + +None. + +### Fabric Link + +#### Example DOM + +##### With href + +```html + + it renders as an anchor tag. + +``` + +##### Without href + +```html + +``` + +#### Considerations + +- `Links` without an `href` provided render as `buttons`. + +### FastDNA Link (Hypertext) + +#### Example DOM + +```html + + Hypertext + +``` + +#### Considerations + +None. + +### Gestalt Link + +#### Example DOM + +```html + + click here + +``` + +#### Considerations + +- Renders by default as a block, and not an inline, element. + +### Material-UI Link + +#### Example DOM + +##### With href + +```html + + Link + +``` + +##### Without href + +```html + +``` + +#### Considerations + +- `Links` without an `href` provided render as `buttons`. + +### Recommended DOM + +After looking at all the component libraries above and taking into consideration common patterns the following DOM is recommended. + +#### For default links + +```html + + {children} + +``` + +#### For recomposed links + +If the link is recomposed to use another tag that is not `a` for its `root` slot, then `role="link"` should be added to the root. An example using `button` can be read below: + +```html + +``` + +### Slots + +From the recommended DOM above we can indicate which slots are going to be required: + +| Name | Considerations | +| ----------- | -------------- | +| `root` | | + +### Considerations that need discussion + +- What about inline vs block links? Should we provide them as well? + - Maybe different styled variant via recomposition. + +## Behaviors + +Aria spec: +https://www.w3.org/TR/wai-aria-1.1/#link +https://www.w3.org/TR/wai-aria-practices/#link + +Fluent UI HIG: +https://microsoft.sharepoint-df.com/:w:/r/teams/OPGUXLeads/_layouts/15/Doc.aspx?sourcedoc=%7BE585806E-01BF-4F37-BF59-12708E4CE81D%7D&file=Links.docx&action=default&mobileredirect=true + +### States + +The following section describes the different states in which a `Link` can be throughout the course of interaction with it. + +#### Enabled state + +An enabled `Link` communicates interaction by having styling that invite the user to click/tap on it to navigate through content. + +#### Disabled state + +A disabled `Link` is non-interactive, disallowing the user to click/tap on it to navigate through content. + +Typically disabled browser elements do now allow focus. This makes the control difficult for a blind user to know about it, or why it's disabled, without scanning the entire page. Therefore it is recommended to allow focus on disabled components and to make them readonly. This means we use `ariaDisabled` attributes, and not `disabled` attributes, for defining a disabled state. This may sometimes require special attention to ignoring input events in the case a browser element might do something. In the past we've introduced an `allowDisabledFocus` prop for component users to control this behavior. + +#### Hovered state + +A hovered `Link` changes styling to communicate that the user has placed a cursor above it. + +#### Focused state + +A focused `Link` changes styling to communicate that the user has placed keyboard focus on it. This styling is usually the same to the one in the hovered state plus extra styling on the outline to indicate keyboard focus has been placed on the component. + +#### States that need discussion + +None. + +### Keyboard interaction + +The following is a set of keys that interact with the `Link` component: + +| Key | Description | +| ------------------------ | --------------------------------------------------------- | +| `Enter` | Executes the `Link` and moves focus to the `Link` target. | +| `Shift + F10` (Optional) | Opens a context menu for the `Link`. | + +### Cursor interaction + +Test: Possible to use this to capture mouse, though Safari does not have compatibility: +https://developer.mozilla.org/en-US/docs/Web/API/Element/setPointerCapture + +- `mouseenter`: Should immediately change the styling of the `Link` so that it appears to be hovered. +- `mouseleave`: Should immediately remove the hovered styling of the `Link`. +- `mouseup`: If triggered while cursor is still inside of the `Link's` boundaries, then it should execute the `Link` and move focus to the `Link` target. + +### Touch interaction + +The same behavior as above translated for touch events. This means that there is no equivalent for `mouseenter` and `mouseleave`, which makes it so that the hovered state cannot be accessed. + +### Screen reader accessibility + +#### `root`: + +- Should default to render a native `a` element unless another `root` slot has been specified. +- Should mix in the native props expected for the `a` native element. +- Should be keyboard tabbable and focusable. + +#### Accessibility concerns for the user. + +The `ariaLabel`, `ariaLabelledby` and `ariaDescribedBy` properties are surfaced to the component interface but are required to be set by the component user to meet accessibility requirements. + +## Themability and customization + +### Composition + +The `Link` component uses `react-texture` to provide a recomposable implementation that has no runtime performance penalties. The `BaseLink` implementation can be used to provide new `slots` and default `props` without the application of additional styling: + +```tsx +const FooLink = BaseLink.compose({ + tokens: {}, + styles: {}, + slots: {} +}); + +render () { + + Go to bing! + +} +``` + +## Class names + +1 per slot +1 per state, tagged on root + +### Component design tokens + +> Tokens represent the general look and feel of the various visual slots. Tokens feed into the styling at the right times in the right slot. +> +> Regarding naming conventions, use a camelCased name following this format: +> `{slot}{property}{state (or none for default)}`. For example: `thumbSizeHovered`. +> +> Common property names: `size`, `background`, `color`, `borderRadius` +> +> Common states: `hovered`, `pressed`, `focused`, `checked`, `checkedHovered`, `disabled` + +| Name | Considerations | +| ------------------------ | -------------- | +| `background` | | +| `backgroundDisabled` | | +| `backgroundHovered` | | +| `backgroundPressed` | | +| `backgroundVisited` | | +| `color` | | +| `colorDisabled` | | +| `colorHovered` | | +| `colorPressed` | | +| `colorVisited` | | +| `fontFamily` | | +| `fontSize` | | +| `fontWeight` | | +| `textDecoration` | | +| `textDecorationDisabled` | | +| `textDecorationHovered` | | +| `textDecorationPressed` | | +| `textDecorationVisited` | | + +### To be discussed + +- What do we do about high contrast? Do we provide additional tokens? + +## Use cases + +> TODO: Example use cases + +## Compatibility with other libraries + +> TODO: If this component represents a selected value, how will that be used in an HTML form? Is there a code example to illustrate? + +> TODO: Is it possible this component could be rendered in a focus zone? If so, should the focus model change in that case? diff --git a/specs/Slider.md b/specs/Slider.md new file mode 100644 index 0000000000..7adc25c968 --- /dev/null +++ b/specs/Slider.md @@ -0,0 +1,442 @@ +# Slider component specification + +The `Slider` component allows a user to slide a single thumb along a horizontal or vertical axis, representing a min/max range. + +## TODO List + +- For each TODO: + 1. Read it + 2. Do research + 3. Answer questions + 4. Remove TODO +- Search for TODO, there should be none! Delete this section even. +- Review with design crew +- Check into repo where code will live + +## Related variant considerations + +`2DSlider` - allows 2-dimensional sliding, used for color picking or 2d panning. + +## Reference implementations + +https://codesandbox.io/s/sliders-xi0zw + +Fabric Slider [docs](https://developer.microsoft.com/en-us/fabric#/controls/web/slider) + +Stardust Slider [docs](http://localhost:8080/components/slider/definition) + +Material UI Slider [docs](https://material-ui.com/components/slider/) + +BaseUI Slider [docs](https://baseweb.design/components/slider/) + +Chakra Slider [docs](https://chakra-ui.com/slider) + +Carbon Slider [docs](https://www.carbondesignsystem.com/components/slider/code) + +AntD Slider [docs](https://ant.design/components/slider/) + +FastDNA Slider [docs](https://github.com/microsoft/fast-dna/tree/master/packages/fast-components-@fluentui/base/src/slider), [example](https://explore.fast.design/components/slider) + +## Props + +> TODO: Consult the prop wizard to derive consistently defined props. + +| Name | Type | Default value | +| ---- | ---- | ------------- | + + +### Recommended props + +| Name | Type | +| -------------- | ----------------------------------------------------------- | +| as | string | +| className | string | +| defaultValue | number \| number[] | +| disabled | boolean | +| marks | { value: number; label: string; size: 's' \| 'm' \| 'l' }[] | +| max | number | +| min | number | +| name | The form name, is injected on the hidden `input` element. | +| onChange | (ev: Event, value: number) => void | +| originFromZero | boolean | +| step | number | +| value | number \| number[] | +| vertical | boolean | + +To be discussed: + +| Name | Concern | +| ------------------------------------- | ---------------------------------- | +| label/errorDescription | Should be a concern of Form | +| ariaLabel/getA11yValueMessageOnChange | Should this be in `accessibility`? | + +### Fabric Slider props + +https://developer.microsoft.com/en-us/fabric#/controls/web/slider + +| Name | Type | Notes | +| -------------- | -------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- | +| ariaLabel | string | | +| ariaValueText | (value: number) => string | | +| buttonProps | React.HTMLAttributes | | +| className | string | | +| componentRef | RefObject | | +| defaultValue | number | | +| disabled | boolean | | +| label | string | | +| max | number | | +| min | number | | +| onChange | (value: number) => void | This is the wrong signature and should be resolved to be (ev, number) | +| onChanged | (ev, number) => void | This is the correct signature but the wrong prop name. Should be removed | +| originFromZero | boolean | Deprecate in favor of `marks`. | +| showValue | boolean | Note it defaults to true, which should violate a naming convention. `valueHidden` might be a better name | +| snapToStep | boolean | Consider deprecating; this has no value. | +| step | number | The difference between the two adjacent values of the Slider | +| styles | IStyleFunctionOrObject | Should be deprecated in favor of recomposition. | +| theme | ITheme | Should not show up in the public props contract. | +| value | number | | +| valueFormat | (value: number) => string | Could be depreacted; consider slots override? | +| vertical | boolean | | + +### Stardust Slider props + +| Name | Type | Notes | +| --------------------------- | -------------------------------------- | ------------------------------------------------------ | +| accessibility | "sliderBehavior" any | Why would a user need this as a prop? | +| animation | AnimationProp | Why would a user need this as a prop on slider? | +| as | React.ElementType | | +| className | string | | +| defaultValue | string \| number | | +| design | ComponentDesign | What is the use case for this? | +| fluid | boolean | Stretching should be the default, this is unneeded. | +| getA11yValueMessageOnChange | "getA11yValueMessageOnChange" function | Unclear what this specifically does; aria-live polite? | +| input | ShorthandValue | Prop polution. | +| inputRef | Ref | When you can't use an input what then? | +| max | string \| number | | +| min | string \| number | | +| onChange | ComponentEventHandler | Great, this is what we want :) | +| step | string \| number | | +| styles | ComponentSlotStyle | Consider only recomposition | +| value | string \| number | | +| variables | any | Consider only recomposition | +| vertical | boolean | | + +### Differences of Fabric/Stardust to resolve + +| Name | Fx | Recommendation | +| --------------------------- | --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| accessibility | S | Discuss: In its current state it's unclear why a user would ever pass an accessibility behavior in. This is different from every framework. How does this help developers? | +| animation | S | Remove: Why would a user need this as a prop on Slider? Also ambiguous; animation for slider movement or just css on the root? | +| ariaLabel | F | Discuss: Without having the user read aria specs, can we abstract this into `screenReaderDescription` or `accessibility.description`? At minimum we need an example of what should go here. | +| ariaValueText | F | When would a user want to use this? How is it different from `ariaLabel`? | +| buttonProps | F | Replace: Use slotProps/slots. | +| componentRef | F | Discuss: Slider imperative API to set focus and read value. Do we need these? | | +| design | S | Remove: What is the use case for this? | +| fluid | S | Replace: Stretch/fill continer should be the default no? | +| getA11yValueMessageOnChange | S | Discuss: Unclear what this specifically does; aria-live polite? | +| input | S | Replace with slotProps/slots and/or specific props. | +| inputRef | S | Use case unclear. Can we remove this? | +| label | F | Potentially we could remove this if there is a viable alternative; Form, for example. | +| originFromZero | F | Add: valuable use case. | +| showValue | F | Note it defaults to true, which should violate a naming convention. `valueHidden` might be a better name | +| styles | F | Remove: this causes perf problems. Replace with recomposition. | +| theme | F | Remove: makes API surface muddy. | +| valueFormat | F | Discuss: Could be replaced with slot/slot props. | +| variables | S | Remove: this causes perf problems. Replace with recomposition. | + +### Conversion process from Fabric 7 to Fluent UI Slider + +Props being changed: + +> TODO + +Props being removed: + +> TODO + +## Slots + +| Name | Considerations | +| --------- | ------------------------------------------ | +| root | | +| thumb | | +| rail | The line behind the slider thumb and rail. | +| track | The selected area of the slider. | +| mark | Optional mark. | +| markLabel | | +| tooltip | The tooltip rendered above a thumb. | + +## DOM structure + +General considerations: + +- Default should fill container horizontally; the track should touch the edges. + - The thumb may extend beyond the boundaries, but should be compensated for in the container to avoid clipping. +- Focus is set on the thumb, allows for mult range slider selection. + +### Recommended DOM + +```html +
+
+ +
+ +
+ +
+ +
+ +
+``` + +### Fabric Slider example DOM + +- Focus is on the container +- Unneeded extra `div` wrapping the rail +- Current impl missing "marks" +- No tooltip support +- no input + +```html +
+
+ + + +
+
+``` + +### Stardust Slider example DOM + +- Extra wrapper div +- input element to receive focus +- No tooltip on thumb + +```html +
+
+ + + + +
+
+``` + +### MUI Slider + +- focus on thumb, allows for multi range slider +- input element purpose unclear; (type=hidden?) could be for form support + +```html + + + + + + +``` + +## Behaviors + +Aria spec: +https://www.w3.org/TR/wai-aria-1.1/#slider + +Fluent UI HIG: +https://microsoft.sharepoint-df.com/:w:/t/OPGUXLeads/EWgxpDSGgEVInkHf9qIT8tEB9ukfQaYzXbLqc97M_3wDqw?e=d8rC23 + +### Disabled state + +A disabled slider does not allow the user to change the value. All events will be ignored. + +Typically disabled browser elements do now allow focus. This makes the control difficult for a blind user to know about it, or why it's disabled, without scanning the entire page. Therefore it is recommended to allow focus on disabled components and to make them readonly. This means we use `aria-disabled` attributes, and not `disabled` attributes, for defining a disabled state. This may sometimes require special attention to ignoring input events in the case a browser element might do something. + +### Focus indicators + +Focus indicators should not show in mouse or touch interaction; they should only appear when keyboard tabbing/directional keystrokes are pressed, and should disappear when mouse/touch interactions occur. + +### Keyboarding + +Assume left/right keyboard handling flips in RTL, unless specified. + +| Key | Description | +| --------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | +| Up/Right | Increments the value of the slider by the amount specified by the `step` prop. If the `shift` modifier is pressed, increases by 10x the `step` value. | +| Down/Left | Decrements the value of the slider by the amount specified by the `step` prop. If the `shift` modifier is pressed, increases by 10x the `step` value. | +| PageUp | Increments by 10x `step`. Behaves as shift up/right. | +| PageDown | Decrements by 10x `step`. Behaves as shift down/left. | +| Home | Reduces the value to the amount specified by the `min` prop. | +| End | Increases the value to the amount specified in the `max` prop. | + +### Mouse input + +Test: Possible to use this to capture mouse, though Safari does not have compatibility: +https://developer.mozilla.org/en-US/docs/Web/API/Element/setPointerCapture + +- `mousedown` should immediately attempt to set value to the appropriate value. +- `mousemove` should be attached to the window during mousedown to track window-wise move events. Value should be updated appropriately. Note; if a `mousemove` occurs + without the primary button pressed, tracking should cancel and treat the even as a `mouseup`. (Edge case: mouse down, move cursor out of window, release mouse) +- `mouseup` should remove the `mousemove` event. + +### Touch + +The same behavior as above, except that the events should concern + +### Screen reader accessibility + +#### `root`: + +- should render the native element using the `as` prop, defaulting to `div` +- should mix in native props expected for the element type defined in `as`. + +The `thumb` slot: + +- should be focusable via `tabindex=0` +- role set to `slider` +- receives `aria-valuemin` and `aria-valuemax` representing min/max +- receives `aria-valuenow` representing the current value + +### Accessibility concerns for the user + +`aria-label` or `aria-labeledby`: Describe when and why. + +## Themability and customization + +### Composition + +The `Slider` uses `react-texture` to provide a recomposable implementation that has no runtime performance penalties. The `BaseSlider` implementation can be used to provide new `slots` and default `props`: + +```tsx +const FooSlider = BaseSlider.compose({ + tokens: {}, + styles: {}, + slots: {} +}); + +render() { + +} +``` + +## Class names + +1 per slot +1 per state, tagged on root + +### Component design tokens + +> Tokens represent the general look and feel of the various visual slots. Tokens feed into the styling at the right times in the right slot. +> +> Regarding naming conventions, use a camelCased name following this format: +> `{slot}{property}{state (or none for default)}`. For example: `thumbSizeHovered`. +> +> Common property names: `size`, `background`, `color`, `borderRadius` +> +> Common states: `hovered`, `pressed`, `focused`, `checked`, `checkedHovered`, `disabled` + +| Name | Considerations | +| ------------------ | -------------- | +| railBorderColor | | +| railBorderRadius | | +| railBorderWidth | | +| railColor | | +| railColorDisabled | | +| railColorFocused | | +| railColorHovered | | +| railColorPressed | | +| railSize | | +| thumbBorderColor | | +| thumbBorderRadius | | +| thumbBorderWidth | | +| thumbColor | | +| thumbColorDisabled | | +| thumbColorFocused | | +| thumbColorHovered | | +| thumbColorPressed | | +| thumbSize | | +| trackBorderColor | | +| trackBorderRadius | | +| trackBorderWidth | | +| trackColor | | +| trackColorDisabled | | +| trackColorFocused | | +| trackColorHovered | | +| trackColorPressed | | +| trackSize | | + +NOTE! Stardust does not follow this convention. Slider currently uses these tokens: + +``` +activeThumbColor: string +activeThumbHeight: string +activeThumbWidth: string +disabledRailColor: string +disabledThumbColor: string +disabledTrackColor: string +height: string +length: string +railColor: string +railHeight: string +thumbBorderPadding: string +thumbColor: string +thumbHeight: string +thumbWidth: string +trackColor: string +``` + +## Use cases + +> TODO: Example use cases + +## Compatibility with other libraries + +> TODO: If this component represents a selected value, how will that be used in an HTML form? Is there a code example to illustrate? + +> TODO: Is it possible this component could be rendered in a focus zone? If so, should the focus model change in that case? From f7236174fd94eacc0e94650a9bbfa9fa59f7ebe3 Mon Sep 17 00:00:00 2001 From: Aneesha Kommineni Date: Tue, 3 Dec 2019 17:50:58 -0800 Subject: [PATCH 02/11] Addressing feedback in review, fixing some errors in stardust props list, list of prop differences --- specs/Checkbox.md | 73 ++++++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/specs/Checkbox.md b/specs/Checkbox.md index 245edeea28..cb2517514b 100644 --- a/specs/Checkbox.md +++ b/specs/Checkbox.md @@ -18,15 +18,17 @@ Fabric Checkbox [docs](https://developer.microsoft.com/en-us/fabric#/controls/we Stardust Checkbox [docs](https://microsoft.github.io/fluent-ui-react/components/checkbox/definition) +Open UI Checkbox [docs](https://open-ui.org/components/checkbox) + Material UI Checkbox [docs](https://material-ui.com/components/checkboxes/) BaseUI Checkbox [docs](https://baseweb.design/components/Checkbox/) Chakra Checkbox [docs](https://chakra-ui.com/Checkbox) -Cabon Checkbox [docs](https://www.carbondesignsystem.com/components/checkbox/code) +Carbon Checkbox [docs](https://www.carbondesignsystem.com/components/checkbox/code) -AntD Checkbox [docs](https://ant.design/components/Checkbox/) +AntD Checkbox [docs](https://ant.design/components/checkbox/) FastDNA Checkbox [docs](https://explore.fast.design/components/Checkbox) @@ -46,7 +48,7 @@ FastDNA Checkbox [docs](https://explore.fast.design/components/Checkbox) | ariaDescribedBy | string | | ariaLabel | string | | ariaLabelledBy | string | -| as | React.ElementType | +| as | keyof JSX.IntrinsicElements | | checked | boolean | | className | string | | defaultChecked | boolean | @@ -74,8 +76,8 @@ https://developer.microsoft.com/en-us/fabric#/controls/web/checkbox | Name | Type | Notes | | -------------------- | -------------------------------------------------------- | -------------------------------------------------------------------------------| -| ariaLabel | string | | | ariaDescribedBy | string | | +| ariaLabel | string | | | ariaLabelledBy | string | | | ariaPositionInset | number | | | ariaSetSize | number | | @@ -89,7 +91,7 @@ https://developer.microsoft.com/en-us/fabric#/controls/web/checkbox | disabled | boolean | | | indeterminate | boolean | | | inputProps | React.ButtonHTMLAttributes| | -| keytipProps | IKpeytipProps | | +| keytipProps | IKeytipProps | | | label | string | | | onChange | (ev, checked) => void | | | onRenderLabel | IRenderFunction | | @@ -101,33 +103,48 @@ https://developer.microsoft.com/en-us/fabric#/controls/web/checkbox | Name | Type | Notes | | -------------------- | -------------------------------------------------------- | -------------------------------------------------------------------------------| -| animation | AnimationProp | | -| as | React.ElementType | default type is "div" | -| className | string | | -| content | ReactNode | | -| design | ComponentDesign | | -| disableAnimations | boolean | default false | -| overwrite | boolean | default false | -| renderer | Renderer | | -| rtl | boolean | default false | -| styles | ComponentSlotStyle | | -| target | Document | | -| theme | ThemeInput | | -| variables | any | | +| accessibility | Accessibility | | +| animation | AnimationProp | | +| as | React.ElementType | default type is "div" | +| checked | boolean | default false | +| className | string | | +| defaultChecked | boolean | default false | +| design | ComponentDesign | | +| disabled | boolean | default false | +| icon | ShorthandValue | {} | +| label | ShorthandValue | | +| labelPosition | enum (Values: start, end) | "end" | +| onChange | ComponentEventHandler | | +| onClick | ComponentEventHandler | | +| styles | ComponentSlotStyle | | +| toggle | boolean | default false | +| variables | any | | ### Differences of Fabric/Stardust to resolve | Name | Type | Notes | | -------------------- | -------------------------------------------------------- | -------------------------------------------------------------------------------| +| accessibility | Accessibility | | | animation | AnimationProp | | -| disableAnimations | boolean | default false | -| overwrite | boolean | default false | -| renderer | Renderer | | -| variables | any | | -| target | Document | | -| content | ReactNode | | -| ariaPositionInSet | number | if checkbox is in a set, should be up to the user to provide a11y | +| as | React.ElementType | default type is "div" | +| ariaDescribedBy | string | | +| ariaLabel | string | | +| ariaLabelledBy | string | | +| ariaPositionInSet | number | if checkbox is in a set, should be up to the user to provide a11y | | ariaSetSize | number | | +| boxSide | 'start' or 'end' | default 'start' | +| checkmarkIconProps | IIconProps | | +| componentRef | IRefObject | | +| defaultIndeterminate | boolean | | +| indeterminate | boolean | | +| inputProps | React.ButtonHTMLAttributes| | +| keytipProps | IKpeytipProps | | +| labelPosition | enum (Values: start, end) | "end" | +| onRenderLabel | IRenderFunction | | +| onClick | ComponentEventHandler | | +| theme | ITheme | | +| toggle | boolean | default false | +| variables | any | | ### Conversion process from Fabric 7 to Fluent UI Checkbox @@ -301,9 +318,7 @@ render() { ``` ### Composition - -1 per slot -1 per state, tagged on root +TBD ### Component design tokens @@ -385,4 +400,4 @@ The `Checkbox` component may be used within a `Form` component by providing the > TODO: If this component represents a selected value, how will that be used in an HTML form? Is there a code example to illustrate? -> TODO: Is it possible this component could be rendered in a focus zone? If so, should the focus model change in that case? \ No newline at end of file +> TODO: Is it possible this component could be rendered in a focus zone? If so, should the focus model change in that case? From 1a0ac5bf196256539e459391c06c98c2e7f720aa Mon Sep 17 00:00:00 2001 From: Aneesha Kommineni Date: Tue, 3 Dec 2019 17:54:09 -0800 Subject: [PATCH 03/11] Another type, add todo on indeterminate state icon --- specs/Checkbox.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/specs/Checkbox.md b/specs/Checkbox.md index cb2517514b..30cb576bea 100644 --- a/specs/Checkbox.md +++ b/specs/Checkbox.md @@ -138,7 +138,7 @@ https://developer.microsoft.com/en-us/fabric#/controls/web/checkbox | defaultIndeterminate | boolean | | | indeterminate | boolean | | | inputProps | React.ButtonHTMLAttributes| | -| keytipProps | IKpeytipProps | | +| keytipProps | IKeytipProps | | | labelPosition | enum (Values: start, end) | "end" | | onRenderLabel | IRenderFunction | | | onClick | ComponentEventHandler | | @@ -167,6 +167,7 @@ defaultChecked: overloading with checked - can just set default value of checked | input | actual checkbox element - what gets checked/unchecked | | icon | visual checkmark, sideways if indeterminate | | box | wraps input & icon - what actual gets the styling | +TODO: indeterminate state icon - need consensus on what this should look like ## DOM structure From 027a2decebd4dff8a13760b9a690c39582e8612d Mon Sep 17 00:00:00 2001 From: Aneesha Kommineni Date: Tue, 3 Dec 2019 18:05:09 -0800 Subject: [PATCH 04/11] More fixes from spec review feedback: remove vertical --- specs/Checkbox.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/specs/Checkbox.md b/specs/Checkbox.md index 30cb576bea..f4baefc6e8 100644 --- a/specs/Checkbox.md +++ b/specs/Checkbox.md @@ -58,7 +58,6 @@ FastDNA Checkbox [docs](https://explore.fast.design/components/Checkbox) | label | string | | name | string | | onChange | (ev: Event, value: boolean) => void | -| vertical | boolean | Note: rtl, styles, and theme come from compose or the ThemeProvider. And name has been added to support checkbox in form scenarios. @@ -167,8 +166,8 @@ defaultChecked: overloading with checked - can just set default value of checked | input | actual checkbox element - what gets checked/unchecked | | icon | visual checkmark, sideways if indeterminate | | box | wraps input & icon - what actual gets the styling | -TODO: indeterminate state icon - need consensus on what this should look like +TODO: indeterminate state icon - need consensus on what this should look like ## DOM structure General considerations: From a74050265a0ceb9f4048edcc0e3365cbdad68ce8 Mon Sep 17 00:00:00 2001 From: Makoto Morimoto Date: Wed, 4 Dec 2019 13:25:39 -0800 Subject: [PATCH 05/11] Button: Adding Button spec. --- specs/Button.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 specs/Button.md diff --git a/specs/Button.md b/specs/Button.md new file mode 100644 index 0000000000..06b65eadc5 --- /dev/null +++ b/specs/Button.md @@ -0,0 +1,11 @@ +# `@fluentui/base` + +> TODO: description + +## Usage + +``` +const reactBase = require('@fluentui/base'); + +// TODO: DEMONSTRATE API +``` From 8042aba507a7b9a2a25d297cb5de621a76a411e8 Mon Sep 17 00:00:00 2001 From: Makoto Morimoto Date: Wed, 4 Dec 2019 13:27:30 -0800 Subject: [PATCH 06/11] Adding Button spec. --- specs/Button.md | 967 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 962 insertions(+), 5 deletions(-) diff --git a/specs/Button.md b/specs/Button.md index 06b65eadc5..c6324f45f5 100644 --- a/specs/Button.md +++ b/specs/Button.md @@ -1,11 +1,968 @@ -# `@fluentui/base` +# Button component specification -> TODO: description +The `Button` component allows users to commit a change or trigger an action via a single click or tap and is often found inside forms, dialogs, panels and/or pages. -## Usage +## Related variant considerations +The following section documents variants of the component that currently exist in Fabric and identifies variants that exist in other component libraries but don't currently exist in Fabric, documenting which component libraries have those variants. + +### Variants existing in Fabric today + +#### Styling related + +The following are variants that don't affect the functionality of the `Button`, but that have a visually distinct representation. A discussion needs to happen about how we support each of these variants and the effect of our theming story in that decision (separate components, via props, etc). + +- `Action/Link button` +- `Circular button` +- `Compound button` +- `Icon button` +- `Primary button` +- `Secondary/Default button` + +#### Functionality related + +The following are variants that are functionally different from the base `Button` variant. A discussion needs to happen about how much of this functionality belongs to base versus a different variant and about the need of maybe having a different spec for these variants if they are significantly different from the base `Button`. + +- `Menu button` +- `Pressable` +- `Split button` +- `Toogle button` + - > TODO - Need to discuss if functionality belongs to base button or separate variant. + +#### Tied to other components + +The following are variants that exist because of the need of `Buttons` to reside inside other components and, while functionally the same, the styling of these `Buttons` is tightly coupled with the styling of these other components. + +- `Command bar button` +- `Message bar button` + +### Variants not in Fabric but that exist in other component libraries +- `Animated button` + - In Semantic UI +- `Block/Fluid button` + - In Ant Design + - In Semantic UI + - In Shards React + - In Stardust UI +- `Button group/set` + - In Ant Design + - In Atlaskit + - In Base Web + - In Carbon Design + - In Elemental UI + - In Material-UI + - In React Bootstrap + - In Semantic UI + - In Shards React + - In Stardust UI +- `Conditional button` + - In Semantic UI +- `Floating action/Raised button` + - In Material-UI + - In PrimeReact +- `Labelled button` + - In Semantic UI +- `Outlined/Ghost button` + - In Ant Design + - In Carbon Design + - In Chakra UI + - In Elemental UI + - In FastDNA + - In Material-UI + - In Shards React +- `Pill/Round button` + - In Ant Design + - In Base Web + - In PrimeReact + - In Shards React +- `Tertiary button` + - In Base Web + - In Carbon Design + - In React Bootstrap + + +## Reference implementations + +The following section documents links to different UI libraries implementations of Buttons, while also providing a code sandbox with a side by side implementation of them for comparison. + +- [Side-by-side implementations](https://codesandbox.io/s/button-implementations-93x8z) + +- [Ant Design Button docs](https://ant.design/components/button/) + +- [Atlaskit Button docs](https://atlaskit.atlassian.com/packages/core/button) + +- [Base Web Button docs](https://baseweb.design/components/button/) + +- [Carbon Design Button docs](https://www.carbondesignsystem.com/components/button/code) + +- [Chakra UI Button docs](https://chakra-ui.com/button) + +- [Elemental UI Button docs](http://elemental-ui.com/buttons) + +- [Fabric Button docs](https://developer.microsoft.com/en-us/fabric#/controls/web/button) + +- FastDNA Button + - [Docs](https://github.com/microsoft/fast-dna/tree/master/packages/fast-components-react-base/src/button) + - [Example](https://explore.fast.design/components/button) + +- [Gestalt Button docs](https://pinterest.github.io/gestalt/?ref=designrevision.com#/Button) + +- [Grommet Button docs](https://v2.grommet.io/button) + +- [Material-UI Button docs](https://material-ui.com/components/buttons/) + +- [Prime React Button docs](https://www.primefaces.org/primereact/#/button) + +- [React Bootstrap Button docs](https://react-bootstrap.github.io/components/buttons/) + +- [Semantic UI Button docs](https://react.semantic-ui.com/elements/button/) + +- [Shards React Button docs](https://designrevision.com/docs/shards-react/component/button) + +- [Stardust Button docs](https://microsoft.github.io/fluent-ui-react/components/button/definition) + +## Props + +The following section documents the properties that will become part of the new component, as well as the process for mitigating all changes when moving from Fabric and Stardust to Fluent UI. + +> TODO: Consult the prop wizard to derive consistently defined props. + +| Name | Type | Default value | Description | +| ---- | ---- | ------------- | ----------- | + + +### Recommended component props + +The `Button` component should inherit the HTML props of the web `button` so that props like `onClick` and `aria` have the same typings as the native web counterparts. + +| Name | Type | Default value | Description | +| ----------- | :--------------: | :-----------: | -------------------------------------------------------------------------------------- | +| `as` | `string` | | Defines a component that should be used as the root element of the `Button`. | +| `className` | `string` | | Defines an additional classname to provide on the root of the `Button`. | +| `disabled` | `boolean` | `false` | Defines whether the `Button` is in an enabled or disabled state. | +| `href` | `string` | | Defines an href that, if provided, will make the `Button` render as an anchor. | +| `primary` | `boolean` | `false` | Defines whether the visual representation of the `Button` should be emphasized or not. | + +> TODO: Talk about the inheritance of native props, what should we do about them? `Pick`, `Omit`, get all of them? What do we do about slots? Do we defect to `any`? + +### Recommended interface props + +| Name | Type | Default value | Description | +| ------- | :----------: | ------------- | --------------------------- | +| `focus` | `() => void` | | Sets focus on the `Button`. | + +### Props to be discussed + +| Name | Description | Concern | +| -------------------- | -------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | +| `allowDisabledFocus` | Defines whether disabled `Buttons` should be tabbable via keyboard navigation or not. | Should we really redefine standard behavior here? | +| `checked` | Defines whether the `Button` is in a checked state. | Does this belong to the base or to a variant `ToggleButton`? | +| `circular` | Defines whether the `Button` should be rendered as a circle instead of as a rectangle. | Should we still have this prop or should we generate a `CircularButton` variant via recomposition of styles? | +| `primary` | Defines whether the visual representation of the `Button` should be emphasized or not. | Should we still have this prop or should we generate a `PrimaryButton` variant via recomposition of styles? | + +### Fabric Button props + +https://developer.microsoft.com/en-us/fabric#/controls/web/button + +#### IButton interface + +| Name | Type | Notes | +| ------------- | :------------------------------------------------------------------------: | ------------------------------------------------------------------------------------------------------------------------------ | +| `dismissMenu` | `() => void` | Should not be part of the base `Button`. Should be considered for `MenuButton`. Maybe just bring it from the `Menu` interface. | +| `focus` | `() => void` | | +| `openMenu` | `(shouldFocusOnContainer?: boolean, shouldFocusOnMount?: boolean) => void` | Should not be part of the base `Button`. Should be considered for `MenuButton`. Maybe just bring it from the `Menu` interface. | + +#### IButtonProps interface + +| Name | Type | Notes | +| ---------------------------------- | :--------------------------------------------------------------------------------------------------------: | ------------------------------------------------------------------------------------------------------------------------------------------- | +| `allowDisabledFocus` | `boolean` | Should we really redefine standard behavior here? | +| `ariaDescription` | `string` | What purpose does this serve? If anything, belongs to `CompoundButton` and not to base. | +| `ariaHidden` | `boolean` | Should use native `aria-hidden` instead. | +| `ariaLabel` | `string` | Should use native `aria-label` instead. | +| `buttonType` | `ButtonType` | Already deprecated. | +| `checked` | `boolean` | Does this belong to the base or to a variant `ToggleButton`? | +| `className` | `string` | | +| `componentRef` | `IRefObject` | | +| `data` | `any` | What purpose does this serve? Maybe remove? | +| `defaultRender` | `any` | What purpose does this serve? Maybe remove? | +| `description` | `string` | Already deprecated in favor of `secondaryText`. | +| `disabled` | `boolean` | | +| `getClassNames` | `(props) => IButtonClassNames` | Should be deprecated in favor of new composition approach. | +| `getSplitButtonClassNames` | `(props) => IButtonClassNames` | Should not be part of the base `Button`. Should be considered for `SplitButton`. Should be deprecated in favor of new composition approach. | +| `href` | `string` | | +| `iconProps` | `IIconProps` | Should be replaced by `slotProps`. | +| `keytipProps` | `IKeytipProps` | Should be removed until we add `Keytips` in Fluent UI. | +| `menuAs` | `IComponentAs` | Should be deprecated in favor of slot overrides. | +| `menuIconProps` | `IIconProps` | Should not be part of the base `Button` and should be replaced by `slotProps` in `MenuButton`. | +| `menuProps` | `IContextualMenuProps` | Should not be part of the base `Button` and should be replaced by `slotProps` in `MenuButton`. | +| `menuTriggerKeyCode` | `KeyCodes \| null` | Should not be part of the base `Button`. Should be considered for `MenuButton`. | +| `onAfterMenuDismiss` | `() => void` | Should not be part of the base `Button`. Should be considered for `MenuButton`. Maybe rename to `onDismiss`? | +| `onMenuClick` | `(ev?: React.MouseEvent \| React.KeyboardEvent, button?: IButtonProps) => void;` | Should not be part of the base `Button`. Should be considered for `MenuButton`. | +| `onRenderAriaDescription` | `IRenderFunction` | Only keep if we are keeping `ariaDescription`. If keeping it, deprecate in favor of slot overrides. | +| `onRenderChildren` | `IRenderFunction` | Should be removed or deprecated in favor of slot overrides. | +| `onRenderDescription` | `IRenderFunction` | Should not be part of base `Button`. Could be considered for `CompoundButton`. In that case, deprecate in favor of slot overrides. | +| `onRenderIcon` | `IRenderFunction` | Should be deprecated in favor of slot overrides. | +| `onRenderMenuIcon` | `IRenderFunction` | Should not be part of the base `Button`. Should be considered for `MenuButton`. Should be deprecated in favor of slot overrides. | +| `onRenderMenu` | `IRenderFunction` | Should not be part of the base `Button`. Should be considered for `MenuButton`. Should be deprecated in favor of slot overrides. | +| `onRenderText` | `IRenderFunction` | Should be deprecated in favor of slot overrides. | +| `persistMenu` | `boolean` | Should this be handled as part of the menu `slotProps` instead of being a separate prop altogether? | +| `primary` | `boolean` | | +| `primaryActionButtonProps` | `IButtonProps` | Should not be part of the base `Button`. Should be replaced by a slot in `SplitButton`. | +| `primaryDisabled` | `boolean` | Should not be part of the base `Button`. Should be considered for `SplitButton`. | +| `renderPersistedMenuHiddenOnMount` | `boolean` | Already deprecated. | +| `rootProps` | `React.ButtonHTMLAttributes \| React.AnchorHTMLAttributes` | Already deprecated. Should use `slotProps` instead. | +| `secondaryText` | `string` | Should not be part of the base `Button`. Should be replaced by a slot in `CompoundButton`. | +| `split` | `boolean` | Should be deprecated in favor of a `SplitButton` variant. | +| `splitButtonAriaLabel` | `string` | Should not be part of the base `Button`. Should be considered for `SplitButton`. Maybe rename to `secondaryActionAriaLabel`? | +| `splitButtonMenuProps` | `IButtonProps` | Should not be part of the base `Button`. Should be replaced by a slot in `SplitButton`. | +| `styles` | `IButtonStyles` | Should be deprecated in favor of recomposition. | +| `theme` | `ITheme` | Should not show up in the public props contract. | +| `text` | `string` | Should be replaced by a slot. | +| `toggle` | `boolean` | Does this belong to the base or to a variant `ToggleButton`? | +| `toggled` | `boolean` | Already deprecated in favor of `checked`. | +| `uniqueId` | `string \| number` | This is used for keytip support in Fabric. Maybe remove it until we add `Keytips` in Fluent UI? | + +### Stardust Button props + +#### ButtonProps interface + +| Name | Type | Notes | +| --------------- | :----------------------------------: | --------------------------------------------------------------------------------------------------------------------------------- | +| `accessibility` | `Accessibility` | Why would a user need this as a prop? | +| `circular` | `boolean` | | +| `disabled` | `boolean` | | +| `fluid` | `boolean` | Should this be a prop or should the library have a separate `BlockButton` variant? | +| `icon` | `ShorthandValue` | Should be replaced by a slot. | +| `iconOnly` | `boolean` | Should this be a prop or should the library have a separate `IconButton` variant? | +| `iconPosition` | `'before' \| 'after'` | Should be deprecated in favor of view recomposition. | +| `loader` | `ShorthandValue` | What's the use case for this? | +| `loading` | `boolean` | Should be deprecated in favor of recomposition. | +| `onClick` | `ComponentEventHandler` | Should be replaced by the native event signature from which we extend. | +| `onFocus` | `ComponentEventHandler` | Should be replaced by the native event signature from which we extend. | +| `primary` | `boolean` | | +| `secondary` | `boolean` | Does this change styling or is this just the default? | +| `size` | `SizeValue` | Should this prop be provided or is this just a matter that could be solved via styling and recomposition? | +| `text` | `boolean` | Should this be a prop or should the library have a separate `GhostButton` variant? If a prop, should it be named `ghost` instead? | + +### Conversion process from Fabric 7 to Fluent UI Button + +#### IButton interface + +| Name | Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| ------------- | -------------------- | :--------------------: | :--------------: | :-------------------: | +| `dismissMenu` | TBD | ❌ | ❌ | ❌ | +| `focus` | TBD | ❌ | ❌ | ❌ | +| `openMenu` | TBD | ❌ | ❌ | ❌ | + +#### IButtonProps interface + +| Name | Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| ---------------------------------- | ------------------------------------- | :--------------------: | :-------------------------------------: | :-------------------: | +| `allowDisabledFocus` | TBD | ❌ | ❌ | ❌ | +| `ariaDescription` | TBD | ❌ | ❌ | ❌ | +| `ariaHidden` | TBD | ❌ | ❌ | ❌ | +| `ariaLabel` | TBD | ❌ | ❌ | ❌ | +| `buttonType` | Removing as it is already deprecated. | ☑ | No, because prop is already deprecated. | ❌ | +| `checked` | TBD | ❌ | ❌ | ❌ | +| `className` | TBD | ❌ | ❌ | ❌ | +| `componentRef` | TBD | ❌ | ❌ | ❌ | +| `data` | TBD | ❌ | ❌ | ❌ | +| `defaultRender` | TBD | ❌ | ❌ | ❌ | +| `description` | Removing as it is already deprecated. | ☑ | No, because prop is already deprecated. | ❌ | +| `disabled` | TBD | ❌ | ❌ | ❌ | +| `getClassNames` | TBD | ❌ | ❌ | ❌ | +| `getSplitButtonClassNames` | TBD | ❌ | ❌ | ❌ | +| `href` | TBD | ❌ | ❌ | ❌ | +| `iconProps` | TBD | ❌ | ❌ | ❌ | +| `keytipProps` | TBD | ❌ | ❌ | ❌ | +| `menuAs` | TBD | ❌ | ❌ | ❌ | +| `menuIconProps` | TBD | ❌ | ❌ | ❌ | +| `menuProps` | TBD | ❌ | ❌ | ❌ | +| `menuTriggerKeyCode` | TBD | ❌ | ❌ | ❌ | +| `onAfterMenuDismiss` | TBD | ❌ | ❌ | ❌ | +| `onMenuClick` | TBD | ❌ | ❌ | ❌ | +| `onRenderAriaDescription` | TBD | ❌ | ❌ | ❌ | +| `onRenderChildren` | TBD | ❌ | ❌ | ❌ | +| `onRenderDescription` | TBD | ❌ | ❌ | ❌ | +| `onRenderIcon` | TBD | ❌ | ❌ | ❌ | +| `onRenderMenuIcon` | TBD | ❌ | ❌ | ❌ | +| `onRenderMenu` | TBD | ❌ | ❌ | ❌ | +| `onRenderText` | TBD | ❌ | ❌ | ❌ | +| `persistMenu` | TBD | ❌ | ❌ | ❌ | +| `primary` | TBD | ❌ | ❌ | ❌ | +| `primaryActionButtonProps` | TBD | ❌ | ❌ | ❌ | +| `primaryDisabled` | TBD | ❌ | ❌ | ❌ | +| `renderPersistedMenuHiddenOnMount` | Removing as it is already deprecated. | ☑ | No, because prop is already deprecated. | ❌ | +| `rootProps` | Removing as it is already deprecated. | ☑ | No, because prop is already deprecated. | ❌ | +| `secondaryText` | TBD | ❌ | ❌ | ❌ | +| `split` | TBD | ❌ | ❌ | ❌ | +| `splitButtonAriaLabel` | TBD | ❌ | ❌ | ❌ | +| `splitButtonMenuProps` | TBD | ❌ | ❌ | ❌ | +| `styles` | TBD | ❌ | ❌ | ❌ | +| `theme` | TBD | ❌ | ❌ | ❌ | +| `text` | TBD | ❌ | ❌ | ❌ | +| `toggle` | TBD | ❌ | ❌ | ❌ | +| `toggled` | Removing as it is already deprecated. | ☑ | No, because prop is already deprecated. | ❌ | +| `uniqueId` | TBD | ❌ | ❌ | ❌ | + +### Conversion process from Stardust to Fluent UI Button + +#### ButtonProps interface + +| Name | Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| --------------- | -------------------- | :--------------------: | :--------------: | :-------------------: | +| `accessibility` | TBD | ❌ | ❌ | ❌ | +| `circular` | TBD | ❌ | ❌ | ❌ | +| `disabled` | TBD | ❌ | ❌ | ❌ | +| `fluid` | TBD | ❌ | ❌ | ❌ | +| `icon` | TBD | ❌ | ❌ | ❌ | +| `iconOnly` | TBD | ❌ | ❌ | ❌ | +| `iconPosition` | TBD | ❌ | ❌ | ❌ | +| `loader` | TBD | ❌ | ❌ | ❌ | +| `loading` | TBD | ❌ | ❌ | ❌ | +| `onClick` | TBD | ❌ | ❌ | ❌ | +| `onFocus` | TBD | ❌ | ❌ | ❌ | +| `primary` | TBD | ❌ | ❌ | ❌ | +| `secondary` | TBD | ❌ | ❌ | ❌ | +| `size` | TBD | ❌ | ❌ | ❌ | +| `text` | TBD | ❌ | ❌ | ❌ | + +## DOM Structure + +The following section documents the DOM structure for the component from different component library examples and then suggests a recommended DOM taking into consideration common patterns between the libraries reviewed. + +### Ant Design Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- `Children` of `Button` components are rendered inside a `span`. + +### Atlaskit Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- Icons can go before and/or after the `children` via the `iconBefore` and `iconAfter` props. +- Uneeded extra `span` wrapper inside of `button` tag. +- Both `icons` and `children` are wrapped in styled `spans`. + +### Base Web Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- The concept of icon does not exist here, instead you can add _enhancers_ which are effectively a `React.Node` that are placed behind or after the `children` under a styled `div`. + +### Carbon Design Button + +#### Example DOM + +```html + +``` + +#### Considerations + +None. + +### Chakra UI Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- Icons can go before and/or after the `children` via the `leftIcon` and `rightIcon` props. + +### Elemental UI Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- Has no built-in support for icons via props. + +### Fabric Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- Icons via props are only supported on the left of the `children` inside the `Button`. +- Text can be provided via a `text` prop and via `children`. + - If both are provided, only the one provided via the `text` prop is rendered. + - If `children` is not a `string`, then both are rendered. + - Text provided via the `text` prop is rendered inside two nested styled `spans`, one for the text container and one for the actual text. + +### FastDNA Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- Has no built-in support for icons via props. +- Extra styled `span` wraps `children` inside of the `Button`. + +### Gestalt Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- Has no built-in support for icons via props. +- Extends to container's width by default. +- Text has to be provided via a `text` prop as `children` are not rendered. + +### Grommet Button + +#### Example DOM + +```html + ``` -const reactBase = require('@fluentui/base'); -// TODO: DEMONSTRATE API +#### Considerations + +- Icons via props are only supported on the left of the `children` inside the `Button`. +- Text has to be provided via the `label` prop as `children` are not rendered. +- A styled `div` is added in-between the icon and the text to add a _gap_ between them. +- Uneeded extra `div` wrapper inside of `button` tag. + +### Material-UI Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- Icons can go before and/or after the `children` via the `startIcon` and `endIcon` props. +- Both `children` and icons are wrapped inside of a `styled` span. +- An extra `span` is added inside of the `Button` for styling the _ripple effect_ in Material-UI. + +### Prime React Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- Icons can go before and/or after the `children` via the `iconPos` prop. +- Icons are rendered via the `:before` and `content` css props. + +### React Bootstrap Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- Has no built-in support for icons via props. + +### Semantic UI Button + +#### Example DOM + +```html + ``` + +#### Considerations + +- Icons via props are only supported if `children` are not being rendered. +- Icons are rendered via the `:before` and `content` css props. + +### Shard React Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- Has no built-in support for icons via props. + +### Stardust Button + +#### Example DOM + +```html + +``` + +#### Considerations + +- Icons via props are only supported if `children` are not being rendered. + - The icon is rendered inside of a styled `span`. +- Text can be provided via both the `content` prop and `children`. + - Text provided via the `content` prop is rendered inside of a styled `span`. + +### Recommended DOM + +After looking at all the component libraries above and taking into consideration common patterns the following DOM is recommended. + +#### Regular buttons + +```html + +``` + +#### Buttons rendered as links + +```html + + + {children} + + +``` + +### Slots + +From the recommended DOM above we can indicate which slots are going to be required: + +| Name | Considerations | +| ----------- | -------------- | +| `root` | | +| `startIcon` | | +| `endIcon` | | + +### Considerations that need discussion + +- Do we provide both a `startIcon` and `endIcon` as recommended above? + - Alternatively we could provide just an `icon` slot that is always on the left and provide a _"button with icon on the right"_ variant via view recomposition. +- Previously we had a `text` prop. Our recommended DOM above is taking that out in favor of just having `children`. + - Do we want to still have a `text` or `content` prop/slot? + - If we do, where do we place `children` in relation to it? + +## Behaviors + +Aria spec: +https://www.w3.org/TR/wai-aria-1.1/#button +https://www.w3.org/TR/wai-aria-practices/#button + +Fluent UI HIG: +https://microsoft.sharepoint-df.com/:w:/r/teams/OPGUXLeads/_layouts/15/Doc.aspx?sourcedoc=%7B150DD97E-0ECE-460D-B868-8BCB91FCB4BA%7D&file=Buttons.docx&action=default&mobileredirect=true + +### States + +The following section describes the different states in which a `Button` can be throughout the course of interaction with it. + +#### Enabled state + +An enabled `Button` communicates interaction by having styling that invite the user to click/tap on it to trigger an action. + +#### Disabled state + +A disabled `Button` is non-interactive, disallowing the user to click/tap on it to trigger an action. + +Typically disabled browser elements do now allow focus. This makes the control difficult for a blind user to know about it, or why it's disabled, without scanning the entire page. Therefore it is recommended to allow focus on disabled components and to make them readonly. This means we use `aria-disabled` attributes, and not `disabled` attributes, for defining a disabled state. This may sometimes require special attention to ignoring input events in the case a browser element might do something. In the past we've introduced an `allowDisabledFocus` prop for component users to control this behavior. + +#### Hovered state + +A hovered `Button` changes styling to communicate that the user has placed a cursor above it. + +#### Focused state + +A focused `Button` changes styling to communicate that the user has placed keyboard focus on it. This styling is usually the same to the one in the hovered state. + +#### Pressed state + +A pressed `Button` changes styling to communicate that the user has clicked/tapped on it. + +#### States that need discussion + +- Checked state in `Toggle buttons`, `Menu buttons` and `Split buttons`. + +### Keyboard interaction + +The following is a set of keys that interact with the `Button` component: + +| Key | Description | +| ------- | ----------------------------- | +| `Space` | Triggers the `Button's` action. | +| `Enter` | Triggers the `Button's` action. | + +### Cursor interaction + +Test: Possible to use this to capture mouse, though Safari does not have compatibility: +https://developer.mozilla.org/en-US/docs/Web/API/Element/setPointerCapture + +- `mouseenter`: Should immediately change the styling of the `Button` so that it appears to be hovered. +- `mouseleave`: Should immediately remove the hovered styling of the `Button`. +- `mousedown`: Should immediately change the styling of the `Button` so that it appears to be pressed. +- `mouseup`: + - If triggered while cursor is still inside of the `Button's` boundaries, then it should trigger the `Button's` action and immediately remove the pressed styling of the `Button`. + - If triggered outside of the `Button's` boundaries, then it should immmediately remove the pressed styling of the `Button` without triggering the `Button's` action. + +### Touch interaction + +The same behavior as above translated for touch events. This means that there is no equivalent for `mouseenter` and `mouseleave`, which makes it so that the hovered state cannot be accessed. + +### Screen reader accessibility + +#### `root`: + +- Should render the native element using the `as` prop, defaulting to a native `button` element, or a native `a` element if the `href` prop has been set. +- Should mix in the native props expected for the `button` or `a` native elements depending on if the `href` prop has been set. +- Should be keyboard tabbable and focusable. + +#### Accessibility concerns for the user. + +The `aria-label`, `aria-labelledby` and `aria-describedby` properties are surfaced to the component interface but are required to be set by the component user to meet accessibility requirements. + +## Themability and customization + +### Composition + +The `Button` component uses `react-texture` to provide a recomposable implementation that has no runtime performance penalties. The `BaseButton` implementation can be used to provide new `slots` and default `props` without the application of additional styling: + +```tsx +const FooButton = BaseButton.compose({ + tokens: {}, + styles: {}, + slots: {} +}); + +const onClickAlert = () => { + alert('Clicked'); +}; + +render() { + + Click me! + +} +``` + +## Class names + +1 per slot +1 per state, tagged on root + +### Component design tokens + +> Tokens represent the general look and feel of the various visual slots. Tokens feed into the styling at the right times in the right slot. +> +> Regarding naming conventions, use a camelCased name following this format: +> `{slot}{property}{state (or none for default)}`. For example: `thumbSizeHovered`. +> +> Common property names: `size`, `background`, `color`, `borderRadius` +> +> Common states: `hovered`, `pressed`, `focused`, `checked`, `checkedHovered`, `disabled` + +#### Button tokens: + +| Name | Considerations | +| ------------------------ | -------------- | +| `background` | | +| `backgroundDisabled` | | +| `backgroundHovered` | | +| `backgroundPressed` | | +| `borderColor` | | +| `borderColorDisabled` | | +| `borderColorHovered` | | +| `borderColorPressed` | | +| `borderRadius` | | +| `borderStyle` | | +| `borderWidth` | | +| `boxShadow` | | +| `boxShadowDisabled` | | +| `boxShadowHovered` | | +| `boxShadowPressed` | | +| `color` | | +| `colorDisabled` | | +| `colorHovered` | | +| `colorPressed` | | +| `fontFamily` | | +| `fontSize` | | +| `fontWeight` | | +| `height` | | +| `lineHeight` | | +| `margin` | | +| `maxHeight` | | +| `maxWidth` | | +| `minHeight` | | +| `minWidth` | | +| `outlineWidth` | | +| `padding` | | +| `width` | | +| `startIconColor` | | +| `startIconColorDisabled` | | +| `startIconColorHovered` | | +| `startIconColorPressed` | | +| `startIconFontSize` | | +| `startIconFontWeight` | | +| `endIconColor` | | +| `endIconColorDisabled` | | +| `endIconColorHovered` | | +| `endIconColorPressed` | | +| `endIconFontSize` | | +| `endIconFontWeight` | | + +#### Primary Button specific tokens: + +| Name | Considerations | +| ------------------------------- | -------------- | +| `backgroundPrimary` | | +| `backgroundPrimaryDisabled` | | +| `backgroundPrimaryHovered` | | +| `backgroundPrimaryPressed` | | +| `borderColorPrimary` | | +| `borderColorPrimaryDisabled` | | +| `borderColorPrimaryHovered` | | +| `borderColorPrimaryPressed` | | +| `colorPrimary` | | +| `colorPrimaryDisabled` | | +| `colorPrimaryHovered` | | +| `colorPrimaryPressed` | | +| `startIconColorPrimary` | | +| `startIconColorPrimaryDisabled` | | +| `startIconColorPrimaryHovered` | | +| `startIconColorPrimaryPressed` | | +| `endIconColorPrimary` | | +| `endIconColorPrimaryDisabled` | | +| `endIconColorPrimaryHovered` | | +| `endIconColorPrimaryPressed` | | + +NOTE! Stardust does not follow this convention. Their `Button` currently uses these tokens: + +``` +backgroundColor: string +backgroundColorActive: string +backgroundColorDisabled: string +backgroundColorFocus: string +backgroundColorHover: string +borderColor: string +borderColorDisabled: string +borderColorHover: string +borderRadius: string +boxShadow: string +circularBackgroundColor: string +circularBackgroundColorActive: string +circularBackgroundColorFocus: string +circularBackgroundColorHover: string +circularBorderColor: string +circularBorderColorFocus: string +circularBorderColorHover: string +circularBorderRadius: string +circularColor: string +circularColorActive: string +color: string +colorDisabled: string +colorFocus: string +colorHover: string +contentFontSize: string +contentFontWeight: FontWeightProperty +contentLineHeight: string +height: string +loaderBorderSize: string +loaderSize: string +loaderSvgAnimationHeight: string +loaderSvgHeight: string +loadingMinWidth: string +maxWidth: string +minWidth: string +padding: string +sizeSmallContentFontSize: string +sizeSmallContentLineHeight: string +sizeSmallHeight: string +sizeSmallLoaderBorderSize: string +sizeSmallLoaderSvgAnimationHeight: string +sizeSmallLoaderSvgHeight: string +sizeSmallMinWidth: string +sizeSmallPadding: string +textColor: string +textColorDisabled: string +textColorHover: string +textPrimaryColor: string +textPrimaryColorHover: string +``` + +### To be discussed + +- What do we do about high contrast? Do we provide additional tokens? + +## Use cases + +> TODO: Example use cases + +## Compatibility with other libraries + +> TODO: If this component represents a selected value, how will that be used in an HTML form? Is there a code example to illustrate? + +> TODO: Is it possible this component could be rendered in a focus zone? If so, should the focus model change in that case? From a0a690c07ac668f0a8831beac936687349ea2e55 Mon Sep 17 00:00:00 2001 From: "REDMOND\\humorimo" Date: Mon, 9 Dec 2019 13:42:46 -0800 Subject: [PATCH 07/11] Adding Menu spec. --- specs/Menu.md | 676 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 676 insertions(+) create mode 100644 specs/Menu.md diff --git a/specs/Menu.md b/specs/Menu.md new file mode 100644 index 0000000000..bf4e54bb6c --- /dev/null +++ b/specs/Menu.md @@ -0,0 +1,676 @@ +# Menu + +## TODOS +* Get reviews +* Write summary at end +* Checkin to github +* Write prototype +* Finalize structure +* Finalize API +* Final review + +## Menu Basics +A menu is a way of displaying items that a user may be able to interact with. Menus vary quite a bit based on implementation and usage. Some are horizontal, some have collapsable sections, and others are just a list. Likewise menu items are equally varied. Some may just convey information but cannot be interacted with like headers or dividers while others are either buttons, links, or expand into a submenu. + +The w3 specifications for a menu suggest that it is only for user actions, not user input. Most varieties of menus interpret this as being for navigation. + +Menus are often combined with a popover or similar floating component so the menu appears over other items and is subsequently dismissed. + +### MenuItem + +Most menus have a concept of a specific menuitem. It is an integral part of the menu and will have some of its own sections throughout. + +## References + +[CodeSandbox of implemented versions](https://codesandbox.io/s/menuexamples-uybsr) + +* [Aria practices](https://www.w3.org/TR/wai-aria-1.1/#menu) +* [Fabric](https://developer.microsoft.com/en-us/fabric#/controls/web/contextualmenu) +* [Stardust](https://microsoft.github.io/fluent-ui-react/components/menu/definition) +* [Material-ui](https://material-ui.com/components/menus/) +* [Chakra-ui](https://chakra-ui.com/menu) +* [AntDesign](https://ant.design/components/menu/) +* [Jquery-ui](https://jqueryui.com/menu/) +* [Carbon-ui](https://www.carbondesignsystem.com/components/overflow-menu/code) +* [Fast-DNA](https://github.com/microsoft/fast-dna/blob/master/packages/fast-components-react-base/src/context-menu/context-menu.tsx) +* [BluePrintJs](https://blueprintjs.com/docs/#core/components/menu) + +### Notes from references: + +* Most render children, rather than passing a list of items in as props +* Menu is usually a `
    ` with `
  • ` as children +* Some are popout menus rather than just being a list, but most do not have an explicit popout. +* Most have a concept of submenus, either explicitly or as a result of nested lists. +* The line between list and menu is fuzzy. + +## API +### Prop comparison \(Stardust vs Fabric\) +_Note:_ I've skipped some of the boilerplate props like className. + +#### Fabric +##### Menu + +| Prop Name | Type | Description | +| --------- | ---- | ----------- | +| target | Target | Target for positioning | +| directionalHint | DirectionalHint | How the menu should try to align itself | +| gapSpace | number | Distance between menu and target | +| beakWidth | number | Width of the beak | +| useTargetWidth | 'boolean' | If the menu should match the targets width | +| useTargetAsMinWidth | boolean | see above | +| bounds | IRectangle \| (target?: targetWindow?: Window)=>< IRectangle \| undefined | The space that the menu can appear in| +| directionalHintForRTL | DirectionalHint | +| gapSpace | number | The space between the menu and its target | +| beakWidth?| number| The size of the beak that points | +| isBeakVisible?| boolean| | +| coverTarget?| boolean| If the menu should cover its target | +| alignTargetEdge?| boolean| If the menu should be aligned. | +| items| IContextualMenuItem[]| The list of items to be rendered | +| labelElementId?| string| The id of a label element for the menu | +| shouldFocusOnMount?| boolean| | +| shouldFocusOnContainer?| boolean| | +| onDismiss?| Event | callback for what should happen when menu is dismissed | Not needed | +| onItemClick?| (ev?: React.MouseEvent \| React.KeyboardEvent, item?: IContextualMenuItem) => boolean \| void| | +| isSubMenu?| boolean| whether or not the menu is owned by another menu | +| id?| string|| +| ariaLabel?| string|| +| doNotLayer?| boolean|| +| directionalHintFixed?| boolean|| +| onMenuOpened?| (contextualMenu?: IContextualMenuProps) => void| callback for when the menu is opened| +| onMenuDismissed?| (contextualMenu?: IContextualMenuProps) => void| callback for when the menu is completly dismissed | +| calloutProps?| ICalloutProps| passthrough props to the callout | +| title?| string| | +| getMenuClassNames?| (theme: ITheme, className?: string) => IContextualMenuClassNames| styling function | +| onRenderSubMenu?| IRenderFunction| a different way of rendering the submenu | +| onRenderMenuList?| IRenderFunction| a different way of rendering the list of items | +| subMenuHoverDelay?| number| how long it should take for the submenu to open | +| contextualMenuItemAs?| React.ComponentClass | React.StatelessComponent| a different way of rendering each item | +| focusZoneProps?| IFocusZoneProps| passthrough props to control how the focus zone works | +| hidden?| boolean| if the menu should be hidden and not destroyed| +| shouldUpdateWhenHidden?| boolean| | +| delayUpdateFocusOnHover?| boolean| if focus should go to the menu items when they are hovered | + + +##### Menuitem +| Prop Name | Type | Description | +| --------- | ---- | ----------- | +| ariaLabel | string | | +| canCheck | boolean | Whether or not this menu item can be checked | +| checked | boolean | Whether or not this menu item is currently checked.| +| className | string | Additional CSS class to apply to the menu item. | +| componentRef | IRefObject | Optional callback to access the IContextualMenuRenderItem interface. This will get passed down to ContextualMenuItem. | +| customOnRenderListLength | number | When rendering a custom menu component that is passed in, the component might also be a list of elements. We want to keep track of the correct index our menu is using based off of the length of the custom list. It is up to the user to increment the count for their list. | +| data | any | Any custom data the developer wishes to associate with the menu item. | +| disabled | boolean | false | Whether the menu item is disabled | +| href | string | Navigate to this URL when the menu item is clicked. | +| iconProps | IIconProps | Props for the Icon. | +| itemProps | Partial | Optional IContextualMenuItemProps overrides to customize behaviors such as item styling via styles. | +| itemType | ContextualMenuItemType | +| key | string | Unique id to identify the item | +| keytipProps | IKeytipProps | Keytip for this contextual menu item | +| onClick | (ev?: React.MouseEvent | React.KeyboardEvent, item?: IContextualMenuItem) => boolean | void | Callback for when the menu item is invoked. If ev.preventDefault() is called in onClick, the click will not close the menu. Returning true will dismiss the menu even if ev.preventDefault() was called. | +| onMouseDown | (item: IContextualMenuItem, event: React.MouseEvent) => void | A function to be executed on mouse down. This is executed before an onClick event and can be used to interrupt native on click events as well. The click event should still handle the commands. This should only be used in special cases when react and non-react are mixed. | +| onRender | (item: any, dismissMenu: (ev?: any, dismissAll?: boolean) => void) => React.ReactNode | Method to custom render this menu item. For keyboard accessibility, the top-level rendered item should be a focusable element (like an anchor or a button) or have the data-is-focusable property set to true. The function receives a function that can be called to dismiss the menu as a second argument. This can be used to make sure that a custom menu item click dismisses the menu. | +| onRenderIcon | IRenderFunction | Custom render function for the menu item icon | +| primaryDisabled | boolean | false | If the menu item is a split button, this prop disables purely the primary action of the button. | +| rel | string | Link relation setting when using href. If target is _blank, rel is defaulted to a value to prevent clickjacking. | +| role | string | Optional override for the menu button's role. Defaults to menuitem or menuitemcheckbox. | +| secondaryText | string | Seconday description for the menu item to display | +| sectionProps | IContextualMenuSection | Properties to apply to render this item as a section. This prop is mutually exclusive with subMenuProps. | +| split | boolean | false | Whether or not this menu item is a splitButton.| +| subMenuProps | IContextualMenuProps | Properties to apply to a submenu to this item. The ContextualMenu will provide default values for target, onDismiss, isSubMenu, id, shouldFocusOnMount, directionalHint, className, and gapSpace, all of which can be overridden. | +| submenuIconProps | IIconProps | Props for the Icon used for the chevron. | +| target | string | Target window when using href. | +| text | string | Text description for the menu item to display | +| title | string | Optional title for displaying text when hovering over an item. | + +#### Stardust + +##### Menu +| Prop Name | Type | Description | +| --------- | ---- | ----------- | +| accessibility| Accessibility| Determines how the keyboard interacts with the menu | +| activeIndex| number \| string| Index of currently active item | +| defaultActiveIndex| number \| string| Default index of currently active item | +| fluid| boolean| whether or not the menu should fill its container | +| iconOnly| boolean| whether or not the menu only has icons | Not needed | +| items| ShorthandCollection\| The items that are to be rendered in the menu | +| onItemClick| ComponentEventHandler\| Callback for what should happen when an item is clicked | +| pills| boolean| If the items should have rounded edges | not needed | +| pointing| boolean \| 'start' \| 'end'| The direction in which an item should point towards | +| primary| boolean| Determines if the menu is primary or not, changes color | +| secondary| boolean| see above but secondary | +| underlined| boolean| If the items should be underlined | +| vertical| boolean| How the menu should render, vertically or horizontally | +| submenu| boolean| If the menu is a submenu or not | +| indicator| ShorthandValue\| How the submenu icon should look | + +##### Menuitem +| Prop Name | Type | Description | +| --------- | ---- | ----------- | +| accessibility | "menuItemBehavior" any | Accessibility behavior if overridden by the user. | +| active | false \| boolean | A menu item can be active. | +| animation | AnimationProp | Generic animation property that can be used for applying different theme animations. | +| as | "a" React.ElementType | An element type to render as (string or component). | +| className | string | Additional CSS class name(s) to apply. | +| content | ReactNode | Shorthand for primary content. | +| defaultMenuOpen | false \| boolean | Default menu open | +| design | ComponentDesign | | +| disabled | false \| boolean | A menu item can show it is currently unable to be interacted with. | +| icon S | ShorthandValue | Name or shorthand for Menu Item Icon | +| iconOnly | false \| boolean | A menu may have just icons. | +| inSubmenu | false \| boolean | Indicates whether the menu item is part of submenu. | +| index | number | MenuItem index inside Menu. | +| indicator S | ShorthandValue | Shorthand for the submenu indicator. | +| itemPosition | number | MenuItem position inside Menu (skipping separators). | +| itemsCount | number | MenuItem count inside Menu. | +| menu S | ShorthandValue | ShorthandCollection | Shorthand for the submenu. | +| menuOpen | false \| boolean | Indicates if the menu inside the item is open. | +| onActiveChanged | ComponentEventHandler | Callback for setting the current menu item as active element in the menu. | +| onBlur | ComponentEventHandler | Called after item blur. | +| onClick | ComponentEventHandler | Called on click. | +| onFocus | ComponentEventHandler | Called after user's focus. | +| onMenuOpenChange | ComponentEventHandler | Event for request to change 'open' value. | +| pills | false \| boolean | A menu can adjust its appearance to de-emphasize its contents. | +| pointing | boolean \| enum | A menu can point to show its relationship to nearby content. For vertical menu, it can point to the start of the item or to the end. | +| primary | false \| boolean | The menu item can have primary type. | +| secondary | false \| boolean | The menu item can have secondary type. | +| styles | ComponentSlotStyle | Additional CSS styles to apply to the component instance. | +| underlined | false \| boolean | Menu items can by highlighted using underline. | +| variables | any | Override for theme site variables to allow modifications of component styling via themes. | +| vertical | false \| boolean | A vertical menu displays elements vertically. | +| wrapper S | {"as":"li"} ShorthandValue | Shorthand for the wrapper component. | + +### Recommended Props +Most props should be removed in favor of a composition model where the parent component passes in menuItems which contain content styled however they want. + +#### Menu component +Should extend `ul` props + +| Prop Name | Prop Type | Notes | +| -------- | -------- | -------- | +| children | `li | MenuItem | Divider ` | Text | +| orientation | enum: `vertical | horizontal` | Specifies how the menu is oriented. | +| onClick | `onClickHandler` | This is from the root UL props but it's worth noting that there shouldn't be a specific onMenuClick/onMenuItemClick handler. See [Behaviors](#On-Menu-Click) for more | + +#### MenuItem component +Should extend `li` props +Most of the other props exist as HTMLAttributes like `selected`, `checked`, `onClick`. + +| Prop Name | Prop Type | Notes | +| --------- | --------- | ----- | +| children | [Flow content](https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#Flow_content) | No content should have on clicks | + +#### SubmenuItem component +Should extend MenuItems props but omit onClick + +| Prop Name | Prop Type | Notes | +| --------- | -------- | -------- | +| children | [Flow content](https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#Flow_content) | No content should have on clicks | + +#### Submenu component + +| Prop Name | Prop Type | Notes | +| --------- | -------- | -------- | +| children | [Flow content](https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#Flow_content) | No content should have on clicks | +| open | boolean | whether or not this particular menu group is open. | + +#### Submenu component +Should take in MenuProps + + +#### Communication between Menu and MenuItems +To help provide additional information to menu items and their context, the menu should implement react context. This allows for synchronization between menu items. This also allows menu groups to pass their open status down to their children to determine how the sub items are rendered. + +Similarly Submenus should control their open state through context. + +#### Discussion: +This is a large departure from the way that both Stardust and Fabric implement menus but it is more in line with the way a lot of other frameworks menus work. Additionally I believe it gives a lot more flexibility through composition which removes some of the pressure to add many props. + +There should be a lot more discussion to see if this relaxed approach to props is appropriate. Additionally it could make SplitButton menu items difficult to implement. + +### Conversion Plan: +#### Fabric to Fluent: + +##### Menu +| Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| -------------------- | ---------------------- | ---------------- | --------------------- | +| target | ❌ | ❌ | ❌ | +| directionalHint | ❌ | ❌ | ❌ | +| gapSpace | ❌ | ❌ | ❌ | +| beakWidth | ❌ | ❌ | ❌ | +| useTargetWidth | ❌ | ❌ | ❌ | +| useTargetAsMinWidth | ❌ | ❌ | ❌ | +| bounds | ❌ | ❌ | ❌ | +| directionalHintForRTL | ❌ | ❌ | ❌ | +| gapSpace | ❌ | ❌ | ❌ | +| beakWidth?| ❌ | ❌ | ❌ | +| isBeakVisible?| ❌ | ❌ | ❌ | +| coverTarget?| ❌ | ❌ | ❌ | +| alignTargetEdge?| ❌ | ❌ | ❌ | +| items| ❌ | ❌ | ❌ | +| labelElementId?| ❌ | ❌ | ❌ | +| shouldFocusOnMount?| ❌ | ❌ | ❌ | +| shouldFocusOnContainer?| ❌ | ❌ | ❌ | +| onDismiss?| ❌ | ❌ | ❌ | +| onItemClick?| ❌ | ❌ | ❌ | +| isSubMenu?| ❌ | ❌ | ❌ | +| id?| ❌ | ❌ | ❌ | +| ariaLabel?| ❌ | ❌ | ❌ | +| doNotLayer?| ❌ | ❌ | ❌ | +| directionalHintFixed?| ❌ | ❌ | ❌ | +| onMenuOpened?| ❌ | ❌ | ❌ | +| onMenuDismissed?| ❌ | ❌ | ❌ | +| calloutProps?| ❌ | ❌ | ❌ | +| title?| ❌ | ❌ | ❌ | +| getMenuClassNames?| ❌ | ❌ | ❌ | +| onRenderSubMenu?| ❌ | ❌ | ❌ | +| onRenderMenuList?| ❌ | ❌ | ❌ | +| subMenuHoverDelay?| ❌ | ❌ | ❌ | +| contextualMenuItemAs?| ❌ | ❌ | ❌ | +| focusZoneProps?| ❌ | ❌ | ❌ | +| hidden?| ❌ | ❌ | ❌ | +| shouldUpdateWhenHidden?| ❌ | ❌ | ❌ | +| delayUpdateFocusOnHover?| ❌ | ❌ | ❌ | + + +##### Menuitem +| Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| -------------------- | ---------------------- | ---------------- | --------------------- | +| ariaLabel | ❌ | ❌ | ❌ | +| canCheck | ❌ | ❌ | ❌ | +| checked | ❌ | ❌ | ❌ | +| className | ❌ | ❌ | ❌ | +| componentRef | ❌ | ❌ | ❌ | +| customOnRenderListLength | ❌ | ❌ | ❌ | +| data | ❌ | ❌ | ❌ | +| disabled | ❌ | ❌ | ❌ | +| href | ❌ | ❌ | ❌ | +| iconProps | ❌ | ❌ | ❌ | +| itemProps | ❌ | ❌ | ❌ | +| itemType | ❌ | ❌ | ❌ | +| key | ❌ | ❌ | ❌ | +| keytipProps | ❌ | ❌ | ❌ | +| onClick | ❌ | ❌ | ❌ | +| onMouseDown | ❌ | ❌ | ❌ | +| onRender | ❌ | ❌ | ❌ | +| onRenderIcon | ❌ | ❌ | ❌ | +| primaryDisabled | ❌ | ❌ | ❌ | +| rel | ❌ | ❌ | ❌ | +| role | ❌ | ❌ | ❌ | +| secondaryText | ❌ | ❌ | ❌ | +| sectionProps | ❌ | ❌ | ❌ | +| split | ❌ | ❌ | ❌ | +| subMenuProps | ❌ | ❌ | ❌ | +| submenuIconProps | ❌ | ❌ | ❌ | +| target | ❌ | ❌ | ❌ | +| text | ❌ | ❌ | ❌ | +| title | ❌ | ❌ | ❌ | + +#### Stardust to Fluent + +##### Menu + +| Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| -------------------- | ---------------------- | ---------------- | --------------------- | +| accessibility| ❌ | ❌ | ❌ | +| activeIndex| ❌ | ❌ | ❌ | +| defaultActiveIndex| ❌ | ❌ | ❌ | +| fluid| ❌ | ❌ | ❌ | +| iconOnly| ❌ | ❌ | ❌ | +| items| ❌ | ❌ | ❌ | +| onItemClick| ❌ | ❌ | ❌ | +| pills| ❌ | ❌ | ❌ | +| pointing| ❌ | ❌ | ❌ | +| primary| ❌ | ❌ | ❌ | +| secondary| ❌ | ❌ | ❌ | +| underlined| ❌ | ❌ | ❌ | +| vertical| ❌ | ❌ | ❌ | +| submenu| ❌ | ❌ | ❌ | +| indicator| ❌ | ❌ | ❌ | + +##### Menuitem + +| Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| -------------------- | ---------------------- | ---------------- | --------------------- | +| accessibility | ❌ | ❌ | ❌ | +| active | ❌ | ❌ | ❌ | +| animation | ❌ | ❌ | ❌ | +| as | ❌ | ❌ | ❌ | +| className | ❌ | ❌ | ❌ | +| content | ❌ | ❌ | ❌ | +| defaultMenuOpen | ❌ | ❌ | ❌ | +| design | ❌ | ❌ | ❌ | +| disabled | ❌ | ❌ | ❌ | +| icon S | ❌ | ❌ | ❌ | +| iconOnly | ❌ | ❌ | ❌ | +| inSubmenu | ❌ | ❌ | ❌ | +| index | ❌ | ❌ | ❌ | +| indicator S | ❌ | ❌ | ❌ | +| itemPosition | ❌ | ❌ | ❌ | +| itemsCount | ❌ | ❌ | ❌ | +| menu S | ❌ | ❌ | ❌ | +| menuOpen | ❌ | ❌ | ❌ | +| onActiveChanged | ❌ | ❌ | ❌ | +| onBlur | ❌ | ❌ | ❌ | +| onClick | ❌ | ❌ | ❌ | +| onFocus | ❌ | ❌ | ❌ | +| onMenuOpenChange | ❌ | ❌ | ❌ | +| pills | ❌ | ❌ | ❌ | +| pointing | ❌ | ❌ | ❌ | +| primary | ❌ | ❌ | ❌ | +| secondary | ❌ | ❌ | ❌ | +| styles | ❌ | ❌ | ❌ | +| underlined | ❌ | ❌ | ❌ | +| variables | ❌ | ❌ | ❌ | +| vertical | ❌ | ❌ | ❌ | +| wrapper S | ❌ | ❌ | ❌ | + +### Notable things +Based on my recommendations, the MenuItem ends up doing a lot of work compared to the Menu itself. The MenuItem would be responsible for managing whether or not its expanded, has a submenu, and any other state it might have. + +## DOM Structure +### HTML DOM structure +```HTML +
      +
    • {"Item one"}
      • +
      +
    • {"Item two"}
      • +
    • + {"Item Three"} +
        +
      • {"SubItem one"}
        • +
      • {"SubItem two"}
        • +
      +
      • +
    +``` + +### Stardust Dom Structure +Note: Class names removed +```HTML + +``` + +### Office-ui-fabric-react DOM Structure +_Note:_ Some long class names removed +```HTML +
    +
      + + + + +
    +
    +``` + +### Fast-DNA DOM Structure +```HTML + +``` + +#### Comments +Fast-DNA is one of the only menus that doesn't use `li` elements. + +### MaterialUI +_Note:_ Some long class names removed +```HTML + +``` + +### AntDesign DOM Structure +_Note:_ Some long class names removed +```HTML +
      + +
    • + + +
    +``` + +## Recommendations + +### Recommended DOM Structure +```HTML +
    +
    {"Item one"}
    +
    +
    {"Item two"}
    + link1 +
    + {"Item Three"} +
    + + I'm not sure if this is the right role or if it is actually a menu item. It should + probably be related to its parent's item in some way. + <--> +
    + + Note: Does not necessarily need to be part of the same dom + tree, could be floating. + <--> +
    +
    {"SubItem one"}
    +
    {"SubItem two"}
    +
    +
    +
+``` + +### Recommended React Structure +#### Shape when used +```TSX +return + {"Item one"} + + {"Item two"} + + {"Item two"} + + {"SubItem one"} + {"SubItem two"} + + +; +``` +#### Menu +```TSX +function() { + return ( + + + {props.children} + + ); +} +``` + +#### MenuItem +```TSX +function() { + return ( + + {props.children} + ); +} +``` + +#### Submenu +Maybe should be named SubmenuContext? +Should submenu have its own context? +```TSX +function() { +return ( + + {props.children} + ); +} +``` + +#### SubmenuList +```TSX +function() { + const openableContext = React.useContext(OpenableContext); + return (openableContext.open && + + {props.children} + ); +} +``` + +#### SubmenuItem +```TSX +function() { + const openableContext = React.useContext(OpenableContext); + return ( + + {props.children} + ); +} +``` + + +### Slots +#### Menu +Since the Menu is only rendering its children, the only slot necessary is the one for the Root `div`. The rest can be passed in as children variants. + +#### MenuItem +Since the MenuItem is only rendering its children, the only slot necessary is the one for the Root `div`. The rest can be passed in as children. + +#### SubmenuList +Since the SubmenuList is effectively the same as the Menu, it should have only one slot for the Root `div` and for all intents and purposes should behave exactly the same as the Menu slot does. + +#### SubmenuItem +Since the SubmenuItem is effectively the same as the MenuItem, it should have only one slot for the Root `div` and for all intents and purposes should behave exactly the same as the MenuItem slot does. + +### Behaviors + +#### Menu +The menu itself should be a list that renders menu items, it will provide context about its overall state but should not pass that directly into its items as props. + +##### Orientation +The menu should provide some help for orienting its contents either vertically, like a left nav, or horizontally, like a nav bar. Ideally this will just be a prop that gets put into context so each item can decide how it should appear. It's possible that the menu should enforce direction itself. + +##### Focus +By default Focus should go to the tab stops and it's up to each menuItem to decide if it has tabindex=0. An example of a menuItem that shouldn't be focusable would be a Divider. + +However for the menu we should provide a focus wrapper that is able to track focus using role="menuitem" to determine what item should get focused. + +The menu does not need to provide a native way to be controlled without getting focus directly. Like above, there should be a simple utility wrapper that could be written to implement this. + +##### On Menu Click +Some implementations of menu have a general `onClick` function that is applied to all items as a way of getting more information. + +Recommendation: There is not a need to have an `onClick` that is called whenever an item is clicked. Instead the root `menu` should take an `onClick` that will fire if a child's on click does not prevent default. + +#### Menu Items +##### On Item Click +The on item click should be supplied individually by the consuming component + +##### Focus +The menu item should change border and background color on focus. + +##### Hover +The menu item should change border and background color on hover. + +An extra consideration needs to be made for forcing focus into the menu item on hover. Menus on most Microsoft desktop applications work this way. We should ensure there is a way to achieve this behavior through composition or props. + +#### Submenu Items +A submenu item should have all of the same states and behaviors as menu items with the only difference being that submenu items lack an `onClick` callback. + +A submenu item needs to provide a way for menus to open on hover as many submenus have that behavior. + +##### Open +A menu item should have a different state depending on if it is open or not. + +### Theming && customization +The menu should have as few opinions on theming as possible, allowing the items to determine customizations as much as possible. There should also be a way to easily remove the theme entirely from the menu so the items can determine the look and feel. + +#### Menu Tokens + +None needed + +#### Menu Item Tokens +| Token Name | +| ---------- | +| rootBackgroundColor | +| rootBorderColor | +| rootHoverBackgroundColor | +| rootFocusedBackgroundColor | +| rootHoverBorderColor | +| rootFocusedBorderColor | +| rootOutlineColor | +| rootHoverOutlineColor | +| rootFocusedOutlineColor | + +#### Submenu Item Tokens +Extends menu item tokens + +| Token Name | +| ---------- | +| rootOpenBackgroundColor | +| rootOpenBorderColor | + + +### Composition +The menu should consist of a list element, a `div`, which renders individual menu items. Each menu item consists of an element which renders with `role="menuitem"` From c07e5890d8e636f812880cc12cab738c434206bd Mon Sep 17 00:00:00 2001 From: Aneesha Kommineni Date: Mon, 9 Dec 2019 15:02:24 -0800 Subject: [PATCH 08/11] Adding table to document & track conversion process --- specs/Checkbox.md | 83 +++++++++++++++++++++++++++++------------------ 1 file changed, 51 insertions(+), 32 deletions(-) diff --git a/specs/Checkbox.md b/specs/Checkbox.md index f4baefc6e8..82d94b55ab 100644 --- a/specs/Checkbox.md +++ b/specs/Checkbox.md @@ -40,34 +40,6 @@ FastDNA Checkbox [docs](https://explore.fast.design/components/Checkbox) | Name | Type | Default value | | ---- | ---- | ------------- | - -### Recommended props - -| Name | Type | -| -------------- | ----------------------------------------------------------- | -| ariaDescribedBy | string | -| ariaLabel | string | -| ariaLabelledBy | string | -| as | keyof JSX.IntrinsicElements | -| checked | boolean | -| className | string | -| defaultChecked | boolean | -| defaultIndeterminate | boolean | -| disabled | boolean | -| indeterminate | boolean | -| label | string | -| name | string | -| onChange | (ev: Event, value: boolean) => void | - -Note: rtl, styles, and theme come from compose or the ThemeProvider. And name has been added to support checkbox in form scenarios. - -Removing the following two props because the ARIA spec dictates role='checkbox' doesn't need aria-posinset and aria-setsize. These are only valid for role='option' which is only in the case the checkbox is a part of a listbox, which is not something we need to account for in the base component API. If the user does need to provide these two props, slotProps could be used to apply additional props to any slot. - -| Name | Concern | -| ------------------------------------- | ----------------------------------------------------------------- | -| ariaPositionInset | if checkbox is in a set, should be up to the user to provide a11y | -| ariaSetSize | same as above | - ### Fabric Checkbox props https://developer.microsoft.com/en-us/fabric#/controls/web/checkbox @@ -144,13 +116,60 @@ https://developer.microsoft.com/en-us/fabric#/controls/web/checkbox | theme | ITheme | | | toggle | boolean | default false | | variables | any | | + +### Recommended props -### Conversion process from Fabric 7 to Fluent UI Checkbox +| Name | Type | +| -------------- | ----------------------------------------------------------- | +| ariaDescribedBy | string | +| ariaLabel | string | +| ariaLabelledBy | string | +| as | keyof JSX.IntrinsicElements | +| checked | boolean | +| className | string | +| defaultChecked | boolean | +| defaultIndeterminate | boolean | +| disabled | boolean | +| indeterminate | boolean | +| label | string | +| name | string | +| onChange | (ev: Event, value: boolean) => void | + +Note: rtl, styles, and theme come from compose or the ThemeProvider. And name has been added to support checkbox in form scenarios. + +Removing the following two props because the ARIA spec dictates role='checkbox' doesn't need aria-posinset and aria-setsize. These are only valid for role='option' which is only in the case the checkbox is a part of a listbox, which is not something we need to account for in the base component API. If the user does need to provide these two props, slotProps could be used to apply additional props to any slot. -Props being changed: +| Name | Concern | +| ------------------------------------- | ----------------------------------------------------------------- | +| ariaPositionInset | if checkbox is in a set, should be up to the user to provide a11y | +| ariaSetSize | same as above | + +### Conversion process from Fabric 7 to Fluent UI Checkbox -Some props, like style & className, should always go into the root or to the applicable element like name into the box element (replacing input). -Data-attributes will be spread using slotProps. +#### CheckboxProps interface + +| Name | Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| -----------------------------| -------------------- | :--------------------: | :--------------: | :-------------------: | +| `ariaDescribedBy` | TBD | ❌ | ❌ | ❌ | +| `ariaLabel` | TBD | ❌ | ❌ | ❌ | +| `ariaLabelledBy` | TBD | ❌ | ❌ | ❌ | +| `ariaPositionInSet` | Won't be transitioned| ❌ | ❌ | ❌ | +| `ariaSetSize` | Won't be transitioned| ❌ | ❌ | ❌ | +| `boxSide` | TBD | ❌ | ❌ | ❌ | +| `checked` | TBD | ❌ | ❌ | ❌ | +| `checkmarkIconProps` | TBD | ❌ | ❌ | ❌ | +| `className` | TBD | ❌ | ❌ | ❌ | +| `componentRef` | TBD | ❌ | ❌ | ❌ | +| `defaultChecked` | Won't be transitioned| ❌ | ❌ | ❌ | +| `defaultIndetermiante` | TBD | ❌ | ❌ | ❌ | +| `disabled` | TBD | ❌ | ❌ | ❌ | +| `indeterminate` | TBD | ❌ | ❌ | ❌ | +| `keytipProps` | TBD | ❌ | ❌ | ❌ | +| `label` | TBD | ❌ | ❌ | ❌ | +| `onChange` | TBD | ❌ | ❌ | ❌ | +| `onRenderLabel` | TBD | ❌ | ❌ | ❌ | +| `styles` | TBD | ❌ | ❌ | ❌ | +| `theme` | TBD | ❌ | ❌ | ❌ | Props being removed: From f34ca362d9bcba8d65b0329ff417790ffb80f039 Mon Sep 17 00:00:00 2001 From: Aneesha Kommineni Date: Tue, 10 Dec 2019 17:05:24 -0800 Subject: [PATCH 09/11] Adding defaultChecked back in --- specs/Checkbox.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/specs/Checkbox.md b/specs/Checkbox.md index 82d94b55ab..327f23242d 100644 --- a/specs/Checkbox.md +++ b/specs/Checkbox.md @@ -146,7 +146,7 @@ Removing the following two props because the ARIA spec dictates role='checkbox' ### Conversion process from Fabric 7 to Fluent UI Checkbox -#### CheckboxProps interface +#### Fluent Checkbox recommended props interface | Name | Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | | -----------------------------| -------------------- | :--------------------: | :--------------: | :-------------------: | @@ -160,7 +160,7 @@ Removing the following two props because the ARIA spec dictates role='checkbox' | `checkmarkIconProps` | TBD | ❌ | ❌ | ❌ | | `className` | TBD | ❌ | ❌ | ❌ | | `componentRef` | TBD | ❌ | ❌ | ❌ | -| `defaultChecked` | Won't be transitioned| ❌ | ❌ | ❌ | +| `defaultChecked` | TBD | ❌ | ❌ | ❌ | | `defaultIndetermiante` | TBD | ❌ | ❌ | ❌ | | `disabled` | TBD | ❌ | ❌ | ❌ | | `indeterminate` | TBD | ❌ | ❌ | ❌ | @@ -174,8 +174,6 @@ Removing the following two props because the ARIA spec dictates role='checkbox' Props being removed: ariaPoisitionInSet and ariaSetSize - when writing parent component, user should set these on the checkbox. -animations: remnant pattern of semantic UI, don't need animations for checkbox theming -defaultChecked: overloading with checked - can just set default value of checked. ## Slots From ed5a85ef193d90a27c6d52121d30630a6ca19902 Mon Sep 17 00:00:00 2001 From: Aneesha Kommineni Date: Wed, 11 Dec 2019 16:06:19 -0800 Subject: [PATCH 10/11] 1st spec done. :D --- specs/Checkbox.md | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/specs/Checkbox.md b/specs/Checkbox.md index 327f23242d..8525bfea22 100644 --- a/specs/Checkbox.md +++ b/specs/Checkbox.md @@ -134,6 +134,7 @@ https://developer.microsoft.com/en-us/fabric#/controls/web/checkbox | label | string | | name | string | | onChange | (ev: Event, value: boolean) => void | +| labelPosition | start or end | Note: rtl, styles, and theme come from compose or the ThemeProvider. And name has been added to support checkbox in form scenarios. @@ -150,24 +151,23 @@ Removing the following two props because the ARIA spec dictates role='checkbox' | Name | Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | | -----------------------------| -------------------- | :--------------------: | :--------------: | :-------------------: | -| `ariaDescribedBy` | TBD | ❌ | ❌ | ❌ | -| `ariaLabel` | TBD | ❌ | ❌ | ❌ | -| `ariaLabelledBy` | TBD | ❌ | ❌ | ❌ | +| `ariaDescribedBy` | User provided | ❌ | ❌ | ❌ | +| `ariaLabel` | User provided | ❌ | ❌ | ❌ | +| `ariaLabelledBy` | User provided | ❌ | ❌ | ❌ | | `ariaPositionInSet` | Won't be transitioned| ❌ | ❌ | ❌ | | `ariaSetSize` | Won't be transitioned| ❌ | ❌ | ❌ | -| `boxSide` | TBD | ❌ | ❌ | ❌ | -| `checked` | TBD | ❌ | ❌ | ❌ | -| `checkmarkIconProps` | TBD | ❌ | ❌ | ❌ | -| `className` | TBD | ❌ | ❌ | ❌ | -| `componentRef` | TBD | ❌ | ❌ | ❌ | -| `defaultChecked` | TBD | ❌ | ❌ | ❌ | -| `defaultIndetermiante` | TBD | ❌ | ❌ | ❌ | -| `disabled` | TBD | ❌ | ❌ | ❌ | -| `indeterminate` | TBD | ❌ | ❌ | ❌ | +| `boxSide` | No; labelPosition | ❌ | ❌ | ❌ | +| `checked` | Yes - native | ❌ | ❌ | ❌ | +| `checkmarkIconProps` | No | ❌ | ❌ | ❌ | +| `className` | Yes - native | ❌ | ❌ | ❌ | +| `defaultChecked` | Yes | ❌ | ❌ | ❌ | +| `defaultIndetermiante` | Yes | ❌ | ❌ | ❌ | +| `disabled` | Yes - native | ❌ | ❌ | ❌ | +| `indeterminate` | Yes - native | ❌ | ❌ | ❌ | | `keytipProps` | TBD | ❌ | ❌ | ❌ | -| `label` | TBD | ❌ | ❌ | ❌ | -| `onChange` | TBD | ❌ | ❌ | ❌ | -| `onRenderLabel` | TBD | ❌ | ❌ | ❌ | +| `label` | Yes - native | ❌ | ❌ | ❌ | +| `onChange` | Yes - native | ❌ | ❌ | ❌ | +| `onRenderLabel` | No; shorthand | ❌ | ❌ | ❌ | | `styles` | TBD | ❌ | ❌ | ❌ | | `theme` | TBD | ❌ | ❌ | ❌ | From 9bad269499a3d6b182dc8df9ff60bccde8879025 Mon Sep 17 00:00:00 2001 From: Aneesha Kommineni Date: Thu, 12 Dec 2019 14:09:20 -0800 Subject: [PATCH 11/11] Updating some other native props --- specs/Checkbox.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/specs/Checkbox.md b/specs/Checkbox.md index 8525bfea22..04af936b31 100644 --- a/specs/Checkbox.md +++ b/specs/Checkbox.md @@ -149,7 +149,7 @@ Removing the following two props because the ARIA spec dictates role='checkbox' #### Fluent Checkbox recommended props interface -| Name | Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | +| Name | To transition or not?| Property transitioned? | Breaking change? | Codemod/Shim created? | | -----------------------------| -------------------- | :--------------------: | :--------------: | :-------------------: | | `ariaDescribedBy` | User provided | ❌ | ❌ | ❌ | | `ariaLabel` | User provided | ❌ | ❌ | ❌ | @@ -160,11 +160,11 @@ Removing the following two props because the ARIA spec dictates role='checkbox' | `checked` | Yes - native | ❌ | ❌ | ❌ | | `checkmarkIconProps` | No | ❌ | ❌ | ❌ | | `className` | Yes - native | ❌ | ❌ | ❌ | -| `defaultChecked` | Yes | ❌ | ❌ | ❌ | -| `defaultIndetermiante` | Yes | ❌ | ❌ | ❌ | +| `defaultChecked` | Yes - native | ❌ | ❌ | ❌ | +| `defaultIndetermiante` | Yes - native | ❌ | ❌ | ❌ | | `disabled` | Yes - native | ❌ | ❌ | ❌ | | `indeterminate` | Yes - native | ❌ | ❌ | ❌ | -| `keytipProps` | TBD | ❌ | ❌ | ❌ | +| `keytipProps` | Yes - redesign | ❌ | ❌ | ❌ | | `label` | Yes - native | ❌ | ❌ | ❌ | | `onChange` | Yes - native | ❌ | ❌ | ❌ | | `onRenderLabel` | No; shorthand | ❌ | ❌ | ❌ |