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

ToggleGroupControl: Fix active background for zero value #66855

Merged

Conversation

ntsekouras
Copy link
Contributor

@ntsekouras ntsekouras commented Nov 8, 2024

What?

Discovered while working on #66837.

ToggleGroupControl had a logic error that would affect the background of the active item, if its value was zero. Zero is a valid value for this component so this PR just fixes that.

Testing Instructions

  1. Test a ToggleGroupControl with ToggleGroupControlOption having zero value
  2. Observe that in this PR the active background color is set properly.
  3. (Alternatively just check out my Discovered while working on DataViews: API for layout density support #66837 and remove this line of code that was added here.

@ntsekouras ntsekouras requested a review from ajitbohra as a code owner November 8, 2024 06:51
Copy link

github-actions bot commented Nov 8, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ntsekouras <[email protected]>
Co-authored-by: mirka <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@ntsekouras ntsekouras self-assigned this Nov 8, 2024
@ntsekouras ntsekouras added [Type] Bug An existing feature does not function as intended [Package] Components /packages/components labels Nov 8, 2024
@ntsekouras ntsekouras requested review from tyxla, mirka and ciampo and removed request for ajitbohra November 8, 2024 06:55
@ntsekouras ntsekouras changed the title Fix/togglegroupcontrol zero value active item background ToggleGroupControl: Fix active background for zero value Nov 8, 2024
Copy link

github-actions bot commented Nov 8, 2024

Flaky tests detected in 82336b3.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11737501169
📝 Reported issues:

@youknowriad youknowriad added the Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code label Nov 8, 2024
Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

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

Good catch, thank you!

packages/components/CHANGELOG.md Outdated Show resolved Hide resolved
@ntsekouras ntsekouras force-pushed the fix/togglegroupcontrol-zero-value-active-item-background branch from 82336b3 to 0fc0998 Compare November 13, 2024 09:33
@ntsekouras ntsekouras enabled auto-merge (squash) November 13, 2024 09:34
@ntsekouras ntsekouras merged commit 0098e9b into trunk Nov 13, 2024
64 checks passed
@ntsekouras ntsekouras deleted the fix/togglegroupcontrol-zero-value-active-item-background branch November 13, 2024 10:09
@github-actions github-actions bot added this to the Gutenberg 19.7 milestone Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants