feat(ui): replace custom toggle with native input switch polyfill#2998
feat(ui): replace custom toggle with native input switch polyfill#2998tomayac wants to merge 1 commit into
Conversation
Uses the new <input type="checkbox" switch> element, polyfilled via input-switch-polyfill for browsers that don't support it yet, kept live-synced with the accent color and color-mode changes.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
📝 WalkthroughWalkthroughThis PR simplifies the toggle input's styling to rely on the native HTML ChangesNative switch toggle and polyfill
Sequence Diagram(s)sequenceDiagram
participant Browser
participant NuxtPlugin as input-switch-polyfill.client.ts
participant Polyfill as input-switch-polyfill
participant DOM
Browser->>NuxtPlugin: plugin init (defineNuxtPlugin)
NuxtPlugin->>NuxtPlugin: check HTMLInputElement.prototype.switch
alt switch unsupported
NuxtPlugin->>Polyfill: dynamic import()
end
NuxtPlugin->>DOM: query input.switch elements
NuxtPlugin->>DOM: read accent-color from first switch
NuxtPlugin->>DOM: set --switch-accent on all switches
DOM-->>NuxtPlugin: MutationObserver triggers on style/class change
NuxtPlugin->>NuxtPlugin: re-run syncSwitchAccent
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
e18e dependency analysisNo dependency warnings found. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/components/Settings/Toggle.client.vue (1)
57-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
:idshorthand.Vue 3.4+ supports same-name shorthand binding;
:id="id"can be simplified to:id.Based on learnings, "In Vue 3.4 and later, you can use same-name shorthand for attribute bindings... Apply this shorthand in .vue components (notably in Settings/Toggle.client.vue)".
✏️ Suggested tweak
- :id="id" + :id🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/Settings/Toggle.client.vue` around lines 57 - 63, The toggle input in Toggle.client.vue still uses the longer same-name binding for the id attribute; update the checkbox element to use Vue 3.4+ same-name shorthand for the existing id binding. Locate the input in the Toggle component and simplify the current :id binding without changing any other props or behavior.Source: Learnings
app/plugins/input-switch-polyfill.client.ts (1)
1-4: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid awaiting the polyfill import in the plugin chain
defineNuxtPlugin(async () => { ... await import('input-switch-polyfill') })still serialises later plugins while that import is in flight. This only affects browsers that lack native switch support, so the delay is limited to those clients; if that matters, switch to object syntax withparallel: trueor defer the import withvoid import(...).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/plugins/input-switch-polyfill.client.ts` around lines 1 - 4, The plugin in defineNuxtPlugin is still awaiting the input-switch-polyfill import, which blocks later plugins for clients without native switch support. Update input-switch-polyfill.client.ts to avoid awaiting the dynamic import in the plugin chain by using object syntax with parallel: true or by deferring it with void import('input-switch-polyfill') while keeping the HTMLInputElement.prototype check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/plugins/input-switch-polyfill.client.ts`:
- Around line 9-24: The syncSwitchAccent logic only runs on html style/class
mutations, so newly mounted input.switch elements can miss --switch-accent until
something else changes. Update the input-switch-polyfill.client.ts plugin so
syncSwitchAccent is invoked once immediately after setup (on import/mount)
and/or extend the MutationObserver on document.documentElement to also watch
childList changes so late-added switches are resynced.
---
Nitpick comments:
In `@app/components/Settings/Toggle.client.vue`:
- Around line 57-63: The toggle input in Toggle.client.vue still uses the longer
same-name binding for the id attribute; update the checkbox element to use Vue
3.4+ same-name shorthand for the existing id binding. Locate the input in the
Toggle component and simplify the current :id binding without changing any other
props or behavior.
In `@app/plugins/input-switch-polyfill.client.ts`:
- Around line 1-4: The plugin in defineNuxtPlugin is still awaiting the
input-switch-polyfill import, which blocks later plugins for clients without
native switch support. Update input-switch-polyfill.client.ts to avoid awaiting
the dynamic import in the plugin chain by using object syntax with parallel:
true or by deferring it with void import('input-switch-polyfill') while keeping
the HTMLInputElement.prototype check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a08a862e-e2fd-49d3-abf0-bc5be2643953
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
app/components/Settings/Toggle.client.vueapp/plugins/input-switch-polyfill.client.tsnuxt.config.tspackage.json
| const syncSwitchAccent = () => { | ||
| const switches = document.querySelectorAll<HTMLInputElement>('input.switch') | ||
| const [first] = switches | ||
| if (!first) return | ||
| // All switches share the same cascade; read from the first one. | ||
| const color = getComputedStyle(first).getPropertyValue('accent-color').trim() | ||
| if (!color || color === 'auto') return | ||
| for (const el of switches) { | ||
| el.style.setProperty('--switch-accent', color) | ||
| } | ||
| } | ||
|
|
||
| new MutationObserver(syncSwitchAccent).observe(document.documentElement, { | ||
| attributes: true, | ||
| attributeFilter: ['style', 'class'], | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does the input-switch-polyfill npm package automatically detect and upgrade input[switch] elements added to the DOM after initial load, or only elements present at import time?
💡 Result:
The input-switch-polyfill package does not appear to automatically detect and upgrade input[switch] elements added to the DOM after the initial page load [1][2]. The polyfill is designed to progressively enhance existing checkbox elements that utilize the switch attribute [1][3]. It functions by applying styles and managing accessibility roles for elements present when the polyfill is initialized [3][2]. Documentation and implementation patterns for the package focus on initial conditional loading to prevent a Flash of Unstyled Content (FOUC), rather than observing dynamic DOM mutations [1][2]. There is no evidence in the project's documentation or source code structure of a MutationObserver or similar mechanism to monitor for dynamically injected elements [2][4][5][6][7][8].
Citations:
- 1: https://github.com/tomayac/input-switch-polyfill/
- 2: https://github.com/tomayac/input-switch-polyfill
- 3: https://engineered.at/articles/a-polyfill-for-the-html-switch-element
- 4: tomayac/input-switch-polyfill@9e91b01
- 5: tomayac/input-switch-polyfill@111e387
- 6: tomayac/input-switch-polyfill@a4502de
- 7: tomayac/input-switch-polyfill@5d0694b
- 8: tomayac/input-switch-polyfill@f98de02
Re-sync on mount: syncSwitchAccent() only runs on html style/class mutations, so switches mounted later can miss --switch-accent until an unrelated mutation occurs. Call it once after import, or watch childList changes too.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/plugins/input-switch-polyfill.client.ts` around lines 9 - 24, The
syncSwitchAccent logic only runs on html style/class mutations, so newly mounted
input.switch elements can miss --switch-accent until something else changes.
Update the input-switch-polyfill.client.ts plugin so syncSwitchAccent is invoked
once immediately after setup (on import/mount) and/or extend the
MutationObserver on document.documentElement to also watch childList changes so
late-added switches are resynced.
…ate PRs Per review feedback, keeps this PR focused on PWA installability, WCO, and app badging. The Share button and native switch polyfill now live in npmx-dev#2997 and npmx-dev#2998 respectively.
ghostdevv
added
the
needs discussion
2 participants
Summary
Settings/Toggle.client.vuewith the native<input type="checkbox" switch>element.input-switch-polyfillfor browsers that don't yet support the nativeswitchattribute, loaded only when needed, kept live-synced with the accent color and color-mode via a MutationObserver.Split out of #2982 per review feedback to keep that PR focused on PWA/WCO/badging.
Test plan
pnpm test:unit— 1673/1673 passingpnpm run test:types— passingvp lint— 0 errorsknip— clean