Skip to content

Fix issues with focus trap on modal #3020

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
Merged
PVince81 merged 2 commits into from
Aug 12, 2022
Merged

Fix issues with focus trap on modal #3020

PVince81 merged 2 commits into from
Aug 12, 2022

Conversation

@juliusknorr
Copy link
Contributor

@juliusknorr juliusknorr commented Aug 12, 2022 •
edited
Loading

This PR addresses two issues that are not directly visible in the components but with the way how our apps use them.

  • Allow click outside even if the focus trap is set
  • Allow apps (like viewer) to pass additional focus trap elements (like the sidebar) which will then be added to the trap so it can be accessed

Corresponding viewer commit: nextcloud/viewer@a2fb110

@juliusknorr juliusknorr added bug Something isn't working 3. to review Waiting for reviews labels Aug 12, 2022
@juliusknorr juliusknorr added this to the 6.0.0 milestone Aug 12, 2022
PVince81
PVince81 approved these changes Aug 12, 2022
View reviewed changes
Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 nice catch

@PVince81 PVince81 added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 12, 2022
@PVince81 PVince81 merged commit 44e5260 into master Aug 12, 2022
@PVince81 PVince81 deleted the bugfix/noid/focus-trap-modal-sidebar branch August 12, 2022 11:27
@raimund-schluessler raimund-schluessler mentioned this pull request Aug 12, 2022
Merged
@juliusknorr juliusknorr mentioned this pull request Aug 12, 2022
Merged
8 tasks
@skjnldsv skjnldsv mentioned this pull request Aug 18, 2022
Merged
@PVince81
Copy link
Contributor

PVince81 commented Aug 22, 2022

/backport to stable5

@PVince81 PVince81 mentioned this pull request Aug 22, 2022
Closed
9 tasks
@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Aug 22, 2022

The backport to stable5 failed. Please do this backport manually.

@PVince81
Copy link
Contributor

PVince81 commented Aug 22, 2022

/backport to stable5

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Aug 22, 2022
Merged
1 task
@MarcelRobitaille MarcelRobitaille mentioned this pull request Oct 19, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@raimund-schluessler raimund-schluessler Awaiting requested review from raimund-schluessler

@marcoambrosini marcoambrosini Awaiting requested review from marcoambrosini

+1 more reviewer

@PVince81 PVince81 PVince81 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

4. to release Ready to be released and/or waiting for tests to finish bug Something isn't working

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@juliusknorr @PVince81 @skjnldsv