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.

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(NcAppSidebar): move focus to sidebar on open and auto return focus on close #5219

Merged
merged 4 commits into from
Feb 7, 2024

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Feb 7, 2024

☑️ Resolves

See description in commits.

Screenshots

On desktop

  1. Open sidebar
  2. Focus is moved to the sidebar
  3. Close sidebar
  4. Focus is moved to the trigger button
  5. For menu - special case, return to the menu trigger
sidebar-focus-desktop.mp4

On mobile with focus-trap - also works

sidebar-focus-mobile.mp4

When sidebar was open, but a new file or tab was selected — focus is moved to the tab, and the element to return is updated

sidebar-focus-after-open.mp4

When the sidebar is initially open on page load - no focus move

sidebar-focus-on-load.mp4

Also works for apps

sidebar-focus-apps.mp4

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@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 Feb 7, 2024
@ShGKme ShGKme added this to the 8.6.2 milestone Feb 7, 2024
@ShGKme ShGKme self-assigned this Feb 7, 2024
@ShGKme
Copy link
Contributor Author

ShGKme commented Feb 7, 2024

Checking Cypress

Copy link
Contributor

@Pytal Pytal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than minor comments, nice!

@ShGKme ShGKme added enhancement New feature or request and removed bug Something isn't working labels Feb 7, 2024
@ShGKme ShGKme force-pushed the fix/nc-app-sidebar--auto-return-focus branch from c719c88 to 6a9e2fa Compare February 7, 2024 02:51
@ShGKme
Copy link
Contributor Author

ShGKme commented Feb 7, 2024

Checking Cypress

<Transition> didn't work, <transition> works.

@ShGKme ShGKme added the feature: app-sidebar Related to the app-sidebar component label Feb 7, 2024
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works great! Thanks a lot!

@JuliaKirschenheuter JuliaKirschenheuter merged commit 7974b1b into master Feb 7, 2024
18 checks passed
@JuliaKirschenheuter JuliaKirschenheuter deleted the fix/nc-app-sidebar--auto-return-focus branch February 7, 2024 09:28
@ShGKme ShGKme changed the title feat(NcAppSidebar): move focus to sidebar on open and auto return focus on close fix(NcAppSidebar): move focus to sidebar on open and auto return focus on close Feb 7, 2024
@ShGKme ShGKme added bug Something isn't working and removed enhancement New feature or request labels Feb 7, 2024
@ShGKme ShGKme mentioned this pull request Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working feature: app-sidebar Related to the app-sidebar component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants