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
When toggling Demo Banner, do not send focus to a new location outside of the sign-in page forms #7932
When toggling Demo Banner, do not send focus to a new location outside of the sign-in page forms #7932
Conversation
methods: { | ||
toggleBanner() { | ||
this.bannerClosed = !this.bannerClosed; | ||
this.previouslyFocusedElement.focus(); |
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.
Put a guard here, since it's possible that this.previouslyFocusedElement
is undefined.
}, | ||
focusChange(e) { | ||
// We need the element prior to the close button | ||
if (this.$refs.close_button && e.target != this.$refs.close_button.$el) { |
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 extra check for $refs.close_button
can make this feature a little confusing. For example, if I do this:
- At Sign-In screen, click "Close" on demo bar right away.
- Use my keyboard to focus on the "Create Account" button
- Press "More info" on demo bar
- Then focus returns to the username bar.
Do you think you can update this to always have this behavior:
Given The demo banner is in closed state
When I focus on an element `$el` on the sign-in page
And I re-open the demo banner
And I re-close the demo banner
Then focus is returned to `$el`.
Here's a video of the scenario above.
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.
okay I understood I will do it.
… info banner button is clicked and transfer focus to that element once the header is dismissed.
a8c1e73
to
dfd7a9b
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.
Looks good!
Recorded the previous element that holds focus before the close button is clicked and transfer focus to that element once the header is dismissed
…
Contributor Checklist
PR process:
Testing:
Reviewer Checklist
yarn
andpip
)