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

Fix disabled ToggleSwitch behavior #1643

Merged
merged 9 commits into from
Nov 29, 2022
Merged

Conversation

camertron
Copy link
Contributor

@camertron camertron commented Nov 28, 2022

Description

We received a bug report via Slack today showing that disabled ToggleSwitches can still be toggled. We have tests to verify this behavior that are currently passing erroneously. This PR accomplishes the following:

  1. Fixes the tests in question.
  2. Disallows toggling disabled ToggleSwitches.
  3. Fixes a bug causing the wrong on/off state to be submitted on toggle. The form should send a 0 if the switch is being toggled from on -> off, and a 1 if it's being toggled from off -> on.

Integration

Does this change require any updates to code in production?

No. The ToggleSwitch component isn't being used anywhere in prod.

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews

Co-authored with @neall

@changeset-bot
Copy link

changeset-bot bot commented Nov 28, 2022

🦋 Changeset detected

Latest commit: e94cc4a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/view-components Patch

Not sure what this means? Click here to learn what changesets are.

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

@camertron camertron temporarily deployed to review-pr-1643 November 28, 2022 21:39 Inactive
@github-actions github-actions bot added the javascript Pull requests that update Javascript code label Nov 28, 2022
@camertron camertron temporarily deployed to github-pages November 28, 2022 21:44 Inactive
@camertron camertron temporarily deployed to review-pr-1643 November 28, 2022 21:57 Inactive
@camertron camertron temporarily deployed to github-pages November 28, 2022 22:01 Inactive
@camertron camertron marked this pull request as ready for review November 28, 2022 22:08
@camertron camertron requested review from a team and keithamus November 28, 2022 22:08
@camertron camertron temporarily deployed to review-pr-1643 November 28, 2022 22:36 Inactive
@camertron camertron temporarily deployed to github-pages November 28, 2022 22:40 Inactive
@github-actions
Copy link
Contributor

⚠️ Visual differences found

Our visual comparison tests found UI differences. Please review the differences by viewing the files changed tab to ensure that the changes were intentional.

Review visual differences

@camertron camertron temporarily deployed to review-pr-1643 November 28, 2022 22:47 Inactive
@camertron camertron temporarily deployed to github-pages November 28, 2022 22:51 Inactive
Copy link
Member

@jonrohan jonrohan left a comment

Choose a reason for hiding this comment

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

LGTM

@jonrohan jonrohan merged commit 7e20e14 into main Nov 29, 2022
@jonrohan jonrohan deleted the fix_toggle_switch_disabled branch November 29, 2022 04:17
@primer-css primer-css mentioned this pull request Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants