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 issues with focus trap on modal #3020

Merged
merged 2 commits into from
Aug 12, 2022
Merged

Fix issues with focus trap on modal #3020

merged 2 commits into from
Aug 12, 2022

Conversation

juliushaertl
Copy link
Contributor

@juliushaertl juliushaertl 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

@juliushaertl
Disable focus trap on click outside
303e07b 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl
Allow to add additional trap elements to the focus trap
98098ff 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl juliushaertl added bug Something isn't working 3. to review Waiting for reviews labels Aug 12, 2022
@juliushaertl juliushaertl 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
@juliushaertl juliushaertl mentioned this pull request Aug 12, 2022
Merged
8 tasks
@skjnldsv skjnldsv mentioned this pull request Aug 18, 2022
Merged
@PVince81
Copy link
Contributor

/backport to stable5

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

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

@PVince81
Copy link
Contributor

/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

@PVince81 PVince81 PVince81 approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

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

@marcoambrosini marcoambrosini Awaiting requested review from marcoambrosini

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.
3 participants
@juliushaertl @PVince81 @skjnldsv