-
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
Make FocusTrapZone useful for focusing the first item in a virtual list #14606
Make FocusTrapZone useful for focusing the first item in a virtual list #14606
Conversation
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 78d0f87:
|
Asset size changes
Over Tolerance (1024 B) Over Baseline Below Baseline New Removed 1 kB = 1000 B Baseline commit: 9bcc6172721b53c64d04556b2312b26e8c6e0278 (build) |
e6887c5
to
c4b4815
Compare
Perf AnalysisNo significant results to display. All results
|
c4b4815
to
cd32a7e
Compare
cd32a7e
to
bb4914f
Compare
packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx
Outdated
Show resolved
Hide resolved
change/office-ui-fabric-react-2020-08-18-19-00-44-details-list-focus.json
Outdated
Show resolved
Hide resolved
packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.types.ts
Outdated
Show resolved
Hide resolved
bb4914f
to
e325fc2
Compare
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.
Thanks for submitting this fix, but due to work we're currently doing to prepare master
for our version 8 beta release, we're asking contributors to either wait a couple weeks to submit fixes (if it's not urgent) or submit to the new 7.0
branch (if it's urgent). See #15222 for more details.
e325fc2
to
9afcfdd
Compare
9afcfdd
to
7a7c232
Compare
7a7c232
to
b74ac1a
Compare
packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx
Show resolved
Hide resolved
9d1b183
to
eb7e7ec
Compare
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.
see 2 comments
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.
I agree on adding a test but approved for now.
Now that we're publishing beta versions of 8.0 please also port this to the master branch once it merges. |
eb7e7ec
to
6b98020
Compare
Hello @ThomasMichon! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Looks like some API updates are missing |
🎉 Handy links: |
🎉 Handy links: |
…st (microsoft#20413) * Porting microsoft#14606 to v8. * Change files * Removing focus.ts changes. * Delete @fluentui-utilities-a66b0f6c-334f-4416-b397-75393f9d22a7.json
Description of changes
Made some minor updates to
FocusTrapZone
so it can be leveraged to implement a common accessibility requirement: "Select the first row in the List". Unlike previous attempts at solving this issue, such as usinginitialFocusedIndex
onDetailsList
, this approach allows a host app to specify when and how it wants to focus an element, simply by providing the correct selector forfirstFocusableElement
on aFocusTrapZone
and calling.focus()
on its component ref after the content loads. The ideal selector to use is[data-selection-index="0"]
, which is the first content row according to theSelectionZone
. This works well with bothDetailsList
andTilesList
without requiring either component to implement something custom.Focus areas to test
Example pages for
TilesList
andDetailsList
have been updated to include the behavior and showcase its non-invasive impact.