Skip to content

fix: settings toggle should use input - #1049

Merged
danielroe merged 32 commits into
npmx-dev:mainfrom
abbeyperini:fix/1028
Feb 11, 2026
Merged

fix: settings toggle should use input#1049
danielroe merged 32 commits into
npmx-dev:mainfrom
abbeyperini:fix/1028

Conversation

@abbeyperini

@abbeyperiniabbeyperini commented Feb 5, 2026

Copy link
Copy Markdown
Contributor

Update settings page toggle to use an <input type="checkbox" /> instead of a button. Wrap the whole thing in a label and hide the checkbox behind styled toggle, so the click/touch target is pretty huge. Use translate animation and style it to look about the same as before. Use theme color so all color modes are supported. Use locale dir to support right to left languages.

Closes: #1028

@vercel

vercelBot commented Feb 5, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
docs.npmx.devReadyReadyPreview, CommentFeb 11, 2026 7:37pm
npmx.devReadyReadyPreview, CommentFeb 11, 2026 7:37pm
1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
npmx-lunariaIgnoredIgnoredFeb 11, 2026 7:37pm

Request Review

@codecov

codecovBot commented Feb 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 58.82353% with 7 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing linesPatch %Lines
app/components/Settings/Toggle.server.vue55.55%4 Missing ⚠️
app/components/Settings/Toggle.client.vue62.50%2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@knowlerknowler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good change. I noticed these had some affordances I don’t typically associate with switches (i.e. activation on Enter).

Comment threadapp/components/Settings/Toggle.client.vue Outdated
Comment threadapp/components/Settings/Toggle.client.vue Outdated
Comment threadapp/components/Settings/Toggle.client.vue Outdated
@coderabbitai

coderabbitaiBot commented Feb 10, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Replaces the button-based Settings Toggle with a label+native input type="checkbox" implementation using v-model. Internal model declaration now uses definedModel<boolean> with required: true and an id is generated for label association. Templates and styles were rewritten to a CSS-grid layout with toggle-default / toggle-reverse classes, a toggle-background element and a moving thumb implemented with pseudo-elements; inputs are visually hidden but retain role="switch" and ARIA sync. Server-side skeleton template was adjusted; tests now mount with an explicit modelValue: false where applicable.

Possibly related PRs

Suggested reviewers

  • danielroe
  • alexdln
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description check✅ PassedThe pull request description directly aligns with the changeset, detailing the replacement of a button-based toggle with a checkbox-based implementation wrapped in a label, animation styling, theme colour support, and locale direction handling.
Linked Issues check✅ PassedThe pull request successfully addresses all coding requirements from issue #1028: checkbox input replaces button, label wrapper is implemented, focus behaviour is preserved, a11y tests are updated, and CSS selectors enable richer styling options.
Out of Scope Changes check✅ PassedAll modifications remain focused on the toggle component refactoring and related test updates specified in issue #1028; no extraneous changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Comment threadapp/components/Settings/Toggle.client.vue Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Comment threadapp/components/Settings/Toggle.client.vue Outdated
@danielroe

Copy link
Copy Markdown
Member

this is a great change!

just one comment: the radii/position of the dot isn't quite right. here's a comparison

beforeSCR-20260211-baky
afterSCR-20260211-baty

@knowlerknowler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking really good! Works as expected. I’ve got a couple of requests (incorporating the forced color mode styles and using :dir() instead of a class) and a few nits.

I didn’t leave it as a comment, but I also wonder if we should use logical properties for the dimensions (i.e. height, width as block-size, inline-size respectively). Reason being is that grid/flex layout are writing-mode/directionality aware.

Comment threadapp/components/Settings/Toggle.client.vue
Comment threadapp/components/Settings/Toggle.client.vue Outdated
Comment threadapp/components/Settings/Toggle.client.vue Outdated
Comment threadapp/components/Settings/Toggle.client.vue Outdated
Comment threadapp/components/Settings/Toggle.client.vue Outdated
Comment threadapp/components/Settings/Toggle.client.vue Outdated
Comment threadapp/components/Settings/Toggle.client.vue Outdated
Comment threadapp/components/Settings/Toggle.client.vue Outdated
Comment threadapp/components/Settings/Toggle.client.vue Outdated
abbeyperiniand others added 5 commits February 10, 2026 19:15
Co-authored-by: Nathan Knowler <nathan@knowler.dev>
Co-authored-by: Nathan Knowler <nathan@knowler.dev>
Co-authored-by: Nathan Knowler <nathan@knowler.dev>
Co-authored-by: Nathan Knowler <nathan@knowler.dev>
Co-authored-by: Nathan Knowler <nathan@knowler.dev>

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Comment threadapp/components/Settings/Toggle.client.vue
Comment threadapp/components/Settings/Toggle.client.vue Outdated
Comment threadapp/components/Settings/Toggle.client.vue Outdated

@knowlerknowler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working through those forced colours styles, @abbeyperini! LGTM!

@danielroe
danielroe added this pull request to the merge queueFeb 11, 2026
Merged via the queue into npmx-dev:main with commit 0d3ea09Feb 11, 2026
17 checks passed
@github-actions

Copy link
Copy Markdown

Thanks for your first contribution, @abbeyperini! 👏

We'd love to welcome you to the npmx community. Come and say hi on Discord! And once you've joined, visit npmx.wamellow.com to claim the contributor role.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: use input-checkbox for toggle instead of button

3 participants

@abbeyperini@danielroe@knowler