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

DropdownMenuV2: use the Icon component to render radio checks #55964

Merged
merged 2 commits into from Nov 9, 2023

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented Nov 8, 2023

What?

Update the way the experimental version of DropdownMenu renders the radio check icon

Why?

Currently, the icon is not displayed correctly across all browsers (including MacOS Safari)

How?

By refactoring the code to use the Icon component to display the icon:

  • it ensures a more coherent approach within DropdownMenu, since the Icon component is used internally to render the checkbox item's check
  • this also fixes the bug, since the Icon component sets an explicit size for the underlying SVG element

Testing Instructions

  • Open Safari on MacOs
  • Load the "With Radios" Storybook example on Safari
  • Verify that, on trunk, the radio icon doesn't show
  • Apply changes from this PR, and verify that the icon shows correctly

Screenshots or screencast

Before After
Screenshot 2023-11-08 at 12 14 56 Screenshot 2023-11-08 at 12 15 02

@ciampo ciampo self-assigned this Nov 8, 2023
@ciampo ciampo requested review from youknowriad and a team November 8, 2023 16:28
@ciampo ciampo added [Type] Bug An existing feature does not function as intended [Package] Components /packages/components labels Nov 8, 2023
@ciampo ciampo marked this pull request as ready for review November 8, 2023 16:28
<SVG xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">
<Circle cx={ 12 } cy={ 12 } r={ 3 }></Circle>
</SVG>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to make that one an official icon in our icon library maybe? cc @jasmussen

@ciampo ciampo enabled auto-merge (squash) November 9, 2023 11:55
@ciampo ciampo merged commit 1e3dd2a into trunk Nov 9, 2023
50 checks passed
@ciampo ciampo deleted the fix/dropdown-menu-radio-item branch November 9, 2023 12:01
@github-actions github-actions bot added this to the Gutenberg 17.1 milestone Nov 9, 2023
cbravobernal pushed a commit that referenced this pull request Nov 14, 2023
* DropdownMenuV2: use the `Icon` component to render radio checks

* CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[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.

None yet

2 participants