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

feat: add a new switch component #960

Open
wants to merge 69 commits into
base: main
Choose a base branch
from

Conversation

JerryWu1234
Copy link
Contributor

@JerryWu1234 JerryWu1234 commented Sep 13, 2024

What is it?

Switch Component Implementation

This PR implements a headless Switch component with the following features:

Features

  • WAI-ARIA compliant switch pattern using role="switch"
  • Keyboard navigation support (Space and Enter keys)
  • Support for disabled state
  • Two-way data binding with bind:checked signal
  • Configurable through props:
    • disabled
    • checked
    • onChange$
    • onClick$
    • autoFocus
    • defaultChecked

Test Coverage

  • Mouse interaction tests
  • Keyboard accessibility tests (Space, Enter, Tab)
  • Default property tests (checked, defaultChecked, disabled states)
  • ARIA attribute validation

Component Structure

  • SwitchRoot: Main component wrapper
  • SwitchInput: Core switch functionality
  • SwitchLabel: Accessible labeling

Usage Example

<Switch.Root bind:checked={checked}>
  <Switch.Label>Toggle Switch</Switch.Label>
  <Switch.Input />
</Switch.Root>

This implementation follows WAI-ARIA best practices and provides a flexible foundation for custom styling while maintaining accessibility.

Why is it needed?

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have ran pnpm change and documented my changes
  • I have add necessary docs (if needed)
  • Added new tests to cover the fix / functionality

Copy link

changeset-bot bot commented Sep 13, 2024

⚠️ No Changeset found

Latest commit: 96585ec

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

pkg-pr-new bot commented Sep 13, 2024

Open in StackBlitz

npm i https://pkg.pr.new/qwikifiers/qwik-ui@960
npm i https://pkg.pr.new/qwikifiers/qwik-ui/@qwik-ui/headless@960
npm i https://pkg.pr.new/qwikifiers/qwik-ui/@qwik-ui/styled@960
npm i https://pkg.pr.new/qwikifiers/qwik-ui/@qwik-ui/utils@960

commit: 96585ec

@JerryWu1234 JerryWu1234 self-assigned this Nov 14, 2024
@JerryWu1234
Copy link
Contributor Author

Hey Jerry! Just went through the implementation. We're getting close!

A couple of areas of improvement:

  • Clicking outside the thumb (but still on the switch itself), does not toggle the switch
  • When hitting the space key, it will turn on then back off (rather than on like the enter key)
  • Certain accessibility features are missing (https://www.w3.org/WAI/ARIA/apg/patterns/switch/)
  • Every example seems to use a signal bind (this is usually only when the user wants to programmatically change something)

https://kobalte.dev/docs/core/components/switch/

I think this is a good source of inspiration to look at. Along with a set of resources I created here: https://docs.google.com/document/d/18eA8WfrYtdkrRpavyoG8EmT_uzPtzt24ifOOTKk5In8/edit?tab=t.0

For example, I think it makes a lot of sense to have a new component tied to a piece of markup for the thumb, rather than having it be on a pseudo element, since that would follow the principles of composability. The idea that pieces of markup and components can blend in with each other in a straightforward way.

Atomic Design is a good read if you'd like to look further into it.

I think TDD / adding some tests would help a lot to prevent regressions and keep things robust. We need some tests before we can put the switch component in the beta stage.

I done

@JerryWu1234
Copy link
Contributor Author

it's weird, I have no idea with this error. I didn’t delete the tailwind file

onClick$={[handleClickSync$, handleClick$]}
>
<input
{...rest}
Copy link
Collaborator

@thejackshelton thejackshelton Mar 15, 2025

Choose a reason for hiding this comment

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

Looks like we need 4 pieces here:

Switch.Track, Switch.Thumb, Switch.Trigger (a button or div), and Switch.HiddenInput

But this is based on the current structure. I would follow the research process here:

https://qwik.design/contributing/research/

@thejackshelton
Copy link
Collaborator

I think we could merge this in as a draft state if you wanted to start consuming the current version.

With the current feedback above a lot of changes are still needed to be prod ready. (and processes have improved since this PR)

Props to the continued effort on this @JerryWu1234 💪 . Many parts of this were not documented, and have since been documented in qwik.design

I will start writing some docs on form support as well

@thejackshelton
Copy link
Collaborator

Hey Jerry! Here's the new docs on form handling:

https://qwik.design/contributing/forms/

@thejackshelton
Copy link
Collaborator

Hey @JerryWu1234 let's remove this changeset since this would upgrade Qwik UI by a major version

image

@JerryWu1234
Copy link
Contributor Author

Hey @JerryWu1234 let's remove this changeset since this would upgrade Qwik UI by a major version

image

OK

Sign up for free to 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.

2 participants