-
Notifications
You must be signed in to change notification settings - Fork 37
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 FAQGroup autofocus with Alloy issue #629
Conversation
🦋 Changeset detectedLatest commit: bc3f4f6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🟢 No design token changes found |
🟢 No visual differences foundOur visual comparison tests did not find any differences in the UI. |
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.
👍
This issue is only present in development environment probably due the Alloy dev setup...
@stamat the change looks fairly context-agnostic to me, so I think it's fine. Just an FYI for future that we don't wanna put dotcom-specific fixes into Primer Brand, as the library is used outside the monolith too.
Summary
I gated the focus logic via the initialRender ref to move focus on arrow navigation. However, the Alloy actually triggers the render of the FAQGroup component, which causes the initialRender ref to be set to false. This causes the autofocus to the first FAQGroup element and the page is scrolled to it.
This PR changes the logic to gate the focus logic via the hasInteracted state instead of the initialRender ref. This way, the focus logic is only triggered after the user has interacted with the FAQGroup component.
This issue is only present in development environment probably due the Alloy dev setup...
List of notable changes:
FAQGroupProps
type to includehasInteracted
state because the focus logic is gated via thehasInteracted
state instead of theinitialRender
ref.What should reviewers focus on?
This can only be tested on the dotcom pages where the FAQGroup component is used, for instace
/features/copilot
. Arrow navigation should move the focus correctly and the page should not scroll to the first FAQGroup element on initial render.Supporting resources (related issues, external links, etc):
Contributor checklist:
Reviewer checklist: