-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
allow details element to be toggled inside selection and focus zones #25324
allow details element to be toggled inside selection and focus zones #25324
Conversation
Asset size changes
Over Tolerance (1024 B) Over Baseline Below Baseline New Removed 1 kB = 1000 B Baseline commit: ca05778e6856a8f368392a4da0685f68a4cafa3b (build) |
📊 Bundle size report🤖 This report was generated against ca05778e6856a8f368392a4da0685f68a4cafa3b |
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 3c9ee5a:
|
Perf Analysis (
|
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
BaseButton | mount | 1169 | 1171 | 5000 | |
Breadcrumb | mount | 2917 | 2935 | 1000 | |
Checkbox | mount | 2630 | 2612 | 5000 | |
CheckboxBase | mount | 2374 | 2355 | 5000 | |
ChoiceGroup | mount | 4323 | 4435 | 5000 | |
ComboBox | mount | 1206 | 1274 | 1000 | |
CommandBar | mount | 9414 | 9745 | 1000 | |
ContextualMenu | mount | 10885 | 10850 | 1000 | |
DefaultButton | mount | 1366 | 1368 | 5000 | |
DetailsRow | mount | 3456 | 3573 | 5000 | |
DetailsRowFast | mount | 3401 | 3497 | 5000 | |
DetailsRowNoStyles | mount | 3375 | 3426 | 5000 | |
Dialog | mount | 3050 | 2994 | 1000 | |
DocumentCardTitle | mount | 590 | 575 | 1000 | |
Dropdown | mount | 3214 | 3124 | 5000 | |
FocusTrapZone | mount | 2021 | 1988 | 5000 | |
FocusZone | mount | 1890 | 1874 | 5000 | |
GroupedList | mount | 52051 | 59379 | 2 | |
GroupedList | virtual-rerender | 25471 | 25528 | 2 | |
GroupedList | virtual-rerender-with-unmount | 93717 | 92201 | 2 | |
GroupedListV2 | mount | 562 | 532 | 2 | |
GroupedListV2 | virtual-rerender | 544 | 530 | 2 | |
GroupedListV2 | virtual-rerender-with-unmount | 550 | 565 | 2 | |
IconButton | mount | 1885 | 1905 | 5000 | |
Label | mount | 728 | 695 | 5000 | |
Layer | mount | 4266 | 4258 | 5000 | |
Link | mount | 816 | 829 | 5000 | |
MenuButton | mount | 1674 | 1636 | 5000 | |
MessageBar | mount | 2180 | 2318 | 5000 | |
Nav | mount | 3206 | 3260 | 1000 | |
OverflowSet | mount | 1316 | 1331 | 5000 | |
Panel | mount | 2493 | 2436 | 1000 | |
Persona | mount | 1247 | 1266 | 1000 | |
Pivot | mount | 1723 | 1621 | 1000 | |
PrimaryButton | mount | 1437 | 1481 | 5000 | |
Rating | mount | 6882 | 6851 | 5000 | |
SearchBox | mount | 1444 | 1514 | 5000 | |
Shimmer | mount | 2681 | 2885 | 5000 | |
Slider | mount | 2092 | 2085 | 5000 | |
SpinButton | mount | 4669 | 4630 | 5000 | |
Spinner | mount | 801 | 806 | 5000 | |
SplitButton | mount | 3113 | 3077 | 5000 | |
Stack | mount | 855 | 861 | 5000 | |
StackWithIntrinsicChildren | mount | 2256 | 2277 | 5000 | |
StackWithTextChildren | mount | 4767 | 4749 | 5000 | |
SwatchColorPicker | mount | 10559 | 10434 | 5000 | |
TagPicker | mount | 2629 | 2694 | 5000 | |
TeachingBubble | mount | 87683 | 91987 | 5000 | |
Text | mount | 785 | 794 | 5000 | |
TextField | mount | 1580 | 1593 | 5000 | |
ThemeProvider | mount | 1526 | 1519 | 5000 | |
ThemeProvider | virtual-rerender | 1071 | 1083 | 5000 | |
ThemeProvider | virtual-rerender-with-unmount | 2167 | 2149 | 5000 | |
Toggle | mount | 1109 | 1119 | 5000 | |
buttonNative | mount | 537 | 535 | 5000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be adding 'SUMMARY'
rather than 'DETAILS'
, since the <summary>
element is the one that gets keyboard focus and is the clickable toggle?
Added an exception for <details> the same as we have for buttons, inputs, and other interactive elements.
Change the exception from the `details` tag to the `summary` tag as the latter is what actually receives focus events.
d851827
to
3c9ee5a
Compare
* master: (21 commits) fix(react-tabster): make acceptCondition optional as Tabster dont require it (microsoft#25416) chore: adds disableButtonEnhancement on triggers (microsoft#25211) applying package updates feat: Add support for the wbtx whiteboard file extension (microsoft#25346) chore(react-select): migrate to new package structure (microsoft#25359) chore(react-divider): migrate to new package structure (microsoft#25360) fix(docsite): codesandbox exports now working properly for newly migrated v9 packages (microsoft#25388) Website: fix focus border on UHF footer links (microsoft#25389) applying package updates feat: re-export react-table logic hooks (microsoft#25386) chore(react-aria): migrate to new package structure (microsoft#25199) chore(babel-preset-global-context): migrate to new package structure (microsoft#25340) applying package updates fix: Improve Stack's style recalculation performance by selectively applying children selectors (microsoft#25381) applying package updates fix(projects-test): explicitly install next version 12 to fix CI (microsoft#25374) allow details element to be toggled inside selection and focus zones (microsoft#25324) fix(react-persona): Changing persona's versions to pinned versions (microsoft#25367) update fast element and foundation package versions stable (microsoft#25364) chore(keyboard-keys, priority-overflow, react-context-selector, react-conformance-griffel): migrate to new package structure (microsoft#25362) ...
…icrosoft#25324) * allow details element to be toggled inside selection and focus zones Added an exception for <details> the same as we have for buttons, inputs, and other interactive elements. * update detailslist details focus/toggle Change the exception from the `details` tag to the `summary` tag as the latter is what actually receives focus events.
Current Behavior
HTML
<details>
element would not toggle when inside aDetailsList
.New Behavior
<details>
element now toggles with both the enter and space keys.Related Issue(s)
Fixes #22135