fix(web): avoid legacy dropdown id collisions#1958
Conversation
- Purpose: prevent legacy Unraid plugins from breaking the Connect hamburger/user-profile dropdown. - Before: Reka generated ids containing dropdown-, which matched over-broad legacy plugin selectors and caused the menu to be hidden immediately. - Problem: users with plugins like ZFS Master installed could not reliably open the dropdown menu in Connect. - Now: dropdown ids and related ARIA references are normalized away from the conflicting dropdown- segment before those plugins can target them. - How: mountUnifiedApp enables a small DOM compatibility observer and the web test covers id + ARIA rewriting behavior.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Disabled knowledge base sources:
WalkthroughThis change implements backwards compatibility for dropdown menu element IDs by automatically sanitizing legacy Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c54299cbfc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!value || !value.includes(CONFLICTING_ID_SEGMENT)) { | ||
| return; | ||
| } | ||
|
|
||
| element.setAttribute(attributeName, normalizeValue(value)); |
There was a problem hiding this comment.
Scope ID sanitization to Reka dropdown IDs only
The new sanitizer rewrites every id, aria-controls, and aria-labelledby value that contains dropdown-, not just Reka’s reka-dropdown-menu-* IDs. Since mountUnifiedApp() enables this observer for the entire document, unrelated elements can have their IDs mutated (for example plugin DOM with id="...dropdown-..."), while other references like for, href="#...", or JS selectors are left unchanged, causing broken label associations and selector lookups. This should be constrained to the specific Reka dropdown ID patterns to avoid global DOM breakage.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1958 +/- ##
==========================================
+ Coverage 51.74% 51.77% +0.02%
==========================================
Files 1028 1029 +1
Lines 70792 70860 +68
Branches 7881 7897 +16
==========================================
+ Hits 36630 36686 +56
- Misses 34039 34051 +12
Partials 123 123 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
This reverts commit 98d37bb.
Summary
dropdown-segmentRoot Cause
Some legacy Unraid plugins target DOM ids containing
dropdown-. Our Reka dropdown markup uses ids likereka-dropdown-menu-*, which lets those plugins accidentally hide the Connect menu immediately after it renders.Testing
Summary by CodeRabbit