Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: v8 SplitButton and split MenuItem have two touch targets when checkable #28523

Merged
merged 3 commits into from
Jul 14, 2023

Conversation

smhigley
Copy link
Contributor

@smhigley smhigley commented Jul 13, 2023

Previous Behavior

With touch, it was only possible to open the menu, and not possible to check/uncheck the primary button. Generally we require the primary action to also be in the menu, but checkable items generally cannot be checked/unchecked in the menu (and are less common controls in general).

The original impetus for the single-touch-target splitbuttons was also geared more towards icon split buttons like those in the ribbon, where the entire touch target is very small. Icon split buttons should not be toggle splits anyway, so in theory there should be less need for toggle splits to have only one touch target.

New Behavior

Now toggleable splitbuttons and checkable split menuitems have two separate touch targets, and can be checked/unchecked with touch.

This PR also exposes the contextual menu item's split menu button to the a11y tree to make this work for touch screen readers and voice control.

Related Issue(s)

@smhigley smhigley self-assigned this Jul 13, 2023
@smhigley smhigley requested review from a team and khmakoto as code owners July 13, 2023 19:48
@github-actions github-actions bot added this to the July Project Cycle Q3 2023 milestone Jul 13, 2023
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9f64b01:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@fabricteam
Copy link
Collaborator

📊 Bundle size report

🤖 This report was generated against 8d06e6d07e96f556e72b7a597664f14bef933eac

@size-auditor
Copy link

size-auditor bot commented Jul 13, 2023

Asset size changes

⚠️ Insufficient baseline data to detect size changes

Unable to find bundle size details for Baseline commit: 8a9e101

Possible causes

  • The baseline build 8a9e101 is broken
  • The Size Auditor run for the baseline build 8a9e101 was not triggered

Recommendations

  • Please merge your branch for this Pull request with the latest master build and commit your changes once again

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 643 630 5000
Breadcrumb mount 1652 1698 1000
Checkbox mount 1692 1680 5000
CheckboxBase mount 1477 1474 5000
ChoiceGroup mount 2889 2923 5000
ComboBox mount 661 655 1000
CommandBar mount 6367 6330 1000
ContextualMenu mount 14951 14237 1000
DefaultButton mount 760 750 5000
DetailsRow mount 2160 2175 5000
DetailsRowFast mount 2212 2175 5000
DetailsRowNoStyles mount 2066 2016 5000
Dialog mount 2663 2609 1000
DocumentCardTitle mount 213 222 1000
Dropdown mount 2019 1970 5000
FocusTrapZone mount 1136 1153 5000
FocusZone mount 1077 1093 5000
GroupedList mount 41850 41951 2
GroupedList virtual-rerender 20137 19994 2
GroupedList virtual-rerender-with-unmount 50793 50428 2
GroupedListV2 mount 235 222 2
GroupedListV2 virtual-rerender 211 212 2
GroupedListV2 virtual-rerender-with-unmount 216 223 2
IconButton mount 1092 1101 5000
Label mount 332 337 5000
Layer mount 2765 2701 5000
Link mount 406 398 5000
MenuButton mount 947 955 5000
MessageBar mount 21643 21525 5000
Nav mount 1951 1976 1000
OverflowSet mount 785 789 5000
Panel mount 1792 1795 1000
Persona mount 765 756 1000
Pivot mount 883 873 1000
PrimaryButton mount 870 836 5000
Rating mount 4672 4694 5000
SearchBox mount 904 926 5000
Shimmer mount 1893 1914 5000
Slider mount 1351 1371 5000
SpinButton mount 2946 2867 5000
Spinner mount 387 397 5000
SplitButton mount 1831 1842 5000
Stack mount 403 407 5000
StackWithIntrinsicChildren mount 867 853 5000
StackWithTextChildren mount 2636 2634 5000
SwatchColorPicker mount 6123 6189 5000
TagPicker mount 1454 1550 5000
Text mount 383 373 5000
TextField mount 964 952 5000
ThemeProvider mount 828 835 5000
ThemeProvider virtual-rerender 584 583 5000
ThemeProvider virtual-rerender-with-unmount 1279 1259 5000
Toggle mount 601 621 5000
buttonNative mount 194 194 5000

@smhigley smhigley merged commit b8b3ad7 into microsoft:master Jul 14, 2023
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Jul 18, 2023
* master:
  ci(bundlesize): opt out using --since as lightrail doesnt support this behaviour and needs everything build on every PR (microsoft#28545)
  feat: automatically add v9 package stories to public docsite and correctly create codesandbox demo source code (microsoft#28528)
  applying package updates
  Keytips: Align keytipData with visible instance for dupes (microsoft#28522)
  applying package updates
  V8 Fluent2 Theme: Spinner sizes (microsoft#28512)
  feat: add extra-tiny size value to size prop (microsoft#28249)
  feat(react-infobutton): Remove InfoIcon from react-infobutton (microsoft#28534)
  Made Breadcrumb package public (microsoft#28549)
  X bars showing incorrect data when the values are large- bug 8380 (microsoft#28510)
  feat(tools): implement `prepare-initial-release` generator (microsoft#28505)
  feat: release react-breadcrumb to preview (microsoft#28402)
  applying package updates
  fix: v8 SplitButton and split MenuItem have two touch targets when checkable (microsoft#28523)
  fix(react-infobutton): Apply aria-owns only when the popover is open and cleanup infobutton stories (microsoft#28463)
  fix: Pivot overflow role uses tab (microsoft#28409)
  Migrate react-search to preview (microsoft#28531)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants