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 dialog not appearing when another modal is open #387

Merged
merged 1 commit into from
Sep 12, 2022

Conversation

Pytal
Copy link
Collaborator

@Pytal Pytal commented Sep 10, 2022

Fix dialog not appearing from the infinite focus trap loop due to the dialog and modal being mounted as siblings instead of being nested as described in tailwindlabs/headlessui#825 (comment)

The solution is to mount the dialog within the existing modal

Prepending to the previous modal is not enough as NcModal automatically remounts itself to document.body by default https://github.com/nextcloud/nextcloud-vue/blob/v7.0.0-beta.2/src/components/NcModal/NcModal.vue#L555-L561

So to prevent automatic mounting in NcModal null is passed to the container prop

This will show errors in the console that can be dismissed as it has no effect other than preventing automatic remounting but is addressed in nextcloud-libraries/nextcloud-vue#3219

@Pytal Pytal added bug Something isn't working 3. to review Waiting for reviews accessibility labels Sep 10, 2022
@Pytal Pytal added this to the 3.0.1 milestone Sep 10, 2022
@Pytal Pytal self-assigned this Sep 10, 2022
@Pytal Pytal force-pushed the fix/modal-focus-trap-loop branch from f72c044 to 0ae3fc0 Compare September 10, 2022 01:07
@ChristophWurst ChristophWurst merged commit 4f335c9 into master Sep 12, 2022
@delete-merged-branch delete-merged-branch bot deleted the fix/modal-focus-trap-loop branch September 12, 2022 07:57
@skjnldsv
Copy link
Contributor

This required v7.0.0-beta.5

Currently master is broken

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 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants