fix(select): an NcSelect dropdown inside a modal is clickable - #561
Merged
rubenvdlinde merged 1 commit intoSep 6, 2026
Merged
Conversation
All eleven of launchpad's remaining e2e failures, across two specs, were one defect wearing a misleading name — and a second, smaller one behind it. NcSelect defaults `appendToBody` to true, so vue-select moves the open menu to `<body>` and positions it with inline styles. Its z-index reads `var(--vs-dropdown-z-index)`, which nc-vue declares on `body` as 9999. NcModal does not use a fixed z-index at all: it assigns one at RUNTIME, counting up as modals open — measured 10005 for the first and 10010 for the second on one page. Every one of those is above 9999, so the modal always wins and the menu is drawn underneath it. The failure never names the select. `elementFromPoint()` at the option's own centre returns whatever modal content happens to sit over it, so Playwright reported `<p class="dashboard-config__hint"> intercepts pointer events` and a reader goes looking at the hint. That is why this survived three rounds of fixes: the interceptor named in the log is innocent.⚠️ NOT THE SAME BUG as `.dashboard-config__field--select` in DashboardConfigModal.vue, and that rule cannot help. It raises the FIELD so a menu rendered INSIDE the modal clears the fields below it. This menu is not inside the modal at all.⚠️ TWO OBVIOUS FIXES BOTH LOSE, MEASURED RATHER THAN ASSUMED. A plain `.vs__dropdown-menu { z-index }` is overridden by nc-vue's own same-specificity rule in `_plugin-vue_export-helper-*.chunk.css`, which loads after this file: the rule shipped in the bundle and the computed value stayed 9999. Redeclaring the variable on `:root` loses to nc-vue's declaration on `body`, which is more specific for everything inside it. Setting the variable on the MENU beats an inherited value whatever the source order, so this needs no `!important`. THE SUPPORT NOTE WAS SETTLED FOR ONE USER OF THE TWO. `global-setup.ts` records it through the logged-in admin's own preference and the marker is per user, so the suite settled it for exactly one of the people it logs in as. With the z-index fixed, "recipient sees the shared dashboard in their switcher" was the last failure standing, and the call log named `cn-support-dialog` intercepting a toggle it also called visible, enabled and stable. seed.sh now settles it for every seeded account, with occ, because occ is the only thing there that can write another user's preference — the app's own endpoint writes the caller's. That mirrors the firstrunwizard block already above it, which documents this same shape for the same test. Verified against a real instance, not reasoned about: conditional-visibility- editor 13 passed of 13, dashboard-sharing 4 passed of 4. Both were 0 of their CI-failing subsets before. The z-index collision belongs upstream in nextcloud-vue, where the modal counter and the dropdown constant should be related rather than coincidental. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
September 6, 2026 06:28
Contributor
Quality Report — ConductionNL/launchpad @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 527/527 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-06 06:32 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
Sep 6, 2026
…d alone (#562) Correcting my own description in #561. It said nc-vue declares `--vs-dropdown-z-index` on `body` as 9999. That is true of nc-vue's chunk stylesheet, and it is ALSO true of this app's own `css/launchpad.css`, which re-declares it there deliberately under ConductionNL/nextcloud-vue#864 because nc-vue 9.10 moved the declaration onto the select root where an appended menu cannot inherit it. Leaving that out made the new rule look like it replaced the old one. It does not. They fix different halves and removing either brings a different failure back: css/launchpad.css restores INHERITANCE, so the menu receives a value. src/styles/workspace.css raises the VALUE above NcModal, which assigns itself a runtime z-index counting up as modals open — measured 10005 for the first and 10010 for the second. Every one beats 9999, so the #864 workaround alone still leaves the menu under any modal. That matters because #864 is open and ends with "delete both once upstream fixes the scoping". Whoever closes it will read this file, and now finds a line saying the 10100 rule must stay until nc-vue relates the two numbers rather than choosing them independently. Both comments now point at each other. Comment-only. No rule changes. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All eleven remaining failures, one defect
Two specs, one cause — and a second, smaller one behind it.
NcSelectdefaultsappendToBodyto true, so vue-select moves the open menu to<body>and positions it with inline styles. Its z-index readsvar(--vs-dropdown-z-index), which nc-vue declares onbodyas 9999.NcModaldoes not use a fixed z-index at all. It assigns one at runtime, counting up as modals open — measured 10005 for the first and 10010 for the second on one page. Every one of those is above 9999, so the modal always wins and the menu is drawn underneath it.Why it survived three rounds of fixes
The failure never names the select.
elementFromPoint()at the option's own centre returns whatever modal content happens to sit over it, so Playwright reported:and a reader goes looking at the hint. The interceptor named in the log is innocent.
Two obvious fixes both lose — measured, not assumed
.vs__dropdown-menu { z-index: 10100 }_plugin-vue_export-helper-*.chunk.css, which loads after this file. The rule shipped in the bundle and the computed value stayed 9999.:root { --vs-dropdown-z-index: 10100 }body, which is more specific for everything inside it.Setting the variable on the menu beats an inherited value whatever the source order, so this needs no
!important.The support note was settled for one user of two
global-setup.tsrecords it through the logged-in admin's own preference, and the marker is per user — so the suite settled it for exactly one of the people it logs in as.With the z-index fixed, "recipient sees the shared dashboard in their switcher" was the last failure standing, and the call log named
cn-support-dialogintercepting a toggle it also called visible, enabled and stable.seed.shnow settles it for every seeded account, withocc— the only thing there that can write another user's preference, since the app's own endpoint writes the caller's. That mirrors thefirstrunwizardblock already above it, which documents this same shape for this same test.Verified against a real instance, not reasoned about
conditional-visibility-editordashboard-sharingUpstream
The z-index collision belongs in nextcloud-vue, where the modal counter and the dropdown constant should be related rather than coincidental. This keeps the app usable meanwhile.
🤖 Generated with Claude Code