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.

Sign up for GitHub

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

Jump to bottom

fix(NcPopover): emit after-show after focus-trap init to correctly return focus when content set focus initially (e.g. NcEmojiPicker) #6342

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(NcPopover): emit after-show after focus-trap init to correctly return focus when content set focus initially (e.g. NcEmojiPicker) #6342

wants to merge 2 commits into from

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Jan 2, 2025 •
edited
Loading

☑️ Resolves

after-show was triggered before the end of init, including focusing on the elements using focus-trap.

This broke focus-trap auto return focus when NcPopover content focused an element on after-show, making it the element to return focus to.

Same for after-hide.

Additionally, fixed types of related setReturnFocus, which allows function

image

🖼️ Screenshots

🏚️ Before 🏡 After
focus moves to body focus returns to the trigger
before after

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

ShGKme added 2 commits January 2, 2025 18:03
@ShGKme
fix(NcPopover): allow function for setReturnFocus
106d365 
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme
fix(NcPopover): emit after-show/after-hide only after focus-trap
acdd9bc 
`after-show` was triggered before the end of init, including focusing on the elements using `focus-trap`.

This broke `focus-trap` auto return focus when `NcPopover` content focused an element on `after-show`, making it the element to return focus to.

Same for `after-hide`.

Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme ShGKme added bug Something isn't working 3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Jan 2, 2025
@ShGKme ShGKme added this to the 8.21.1 milestone Jan 2, 2025
@ShGKme ShGKme requested review from DorraJaouad and Antreesy January 2, 2025 18:12
@ShGKme ShGKme self-assigned this Jan 2, 2025
@ShGKme ShGKme requested a review from Pytal January 2, 2025 18:12
@ShGKme
Copy link
Contributor Author

ShGKme commented Jan 2, 2025

/backport to next

@ShGKme ShGKme added feature: popover Related to the popovermenu component feature: emoji picker Related to the emoji picker component labels Jan 2, 2025
@ShGKme ShGKme mentioned this pull request Jan 3, 2025
Draft
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DorraJaouad DorraJaouad Awaiting requested review from DorraJaouad

@Antreesy Antreesy Awaiting requested review from Antreesy

@Pytal Pytal Awaiting requested review from Pytal

At least 2 approving reviews are required to merge this pull request.

Assignees

@ShGKme ShGKme

Labels
3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities backport-request bug Something isn't working feature: emoji picker Related to the emoji picker component feature: popover Related to the popovermenu component
Projects
None yet
Milestone
8.21.1
Development

Successfully merging this pull request may close these issues.

[NcEmojiPicker] Focus is lost on close
1 participant
@ShGKme