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

CustomSelectControlV2: keep item checkmark top aligned #63230

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented Jul 8, 2024

What?

Part of #55023
Extracted from #63139

Why?

Design feedback

How?

By using flexbox's alignment options + adding a small margin to preserve the correct vertical alignment with the first line of item text.

Testing Instructions

  1. Apply the following diff
diff --git a/packages/components/src/custom-select-control/stories/index.story.tsx b/packages/components/src/custom-select-control/stories/index.story.tsx
index 8ff9a023c5..9ebf38ab3d 100644
--- a/packages/components/src/custom-select-control/stories/index.story.tsx
+++ b/packages/components/src/custom-select-control/stories/index.story.tsx
@@ -111,7 +111,8 @@ WithHints.args = {
 		{
 			key: 'medium',
 			name: 'Medium',
-			__experimentalHint: '250x250',
+			__experimentalHint:
+				'A very long hint just to test the checkmark alignment (250x250)',
 		},
 		{
 			key: 'large',
  1. Open the CustomSelectControlV2 Legacy "With Hints" example
  2. Select the "Medium" item
  3. Open the select popover, make sure that the checkmark is correctly top aligned
  4. Select other items on this and other examples, make sure that the checkmark is center aligned with the item text when the option item fits on one row

Screenshots or screencast

Scenario Before (trunk) After (this PR)
Selected item on one line Screenshot 2024-07-08 at 10 40 17 Screenshot 2024-07-08 at 10 43 28
Selected item on multiple lines Screenshot 2024-07-08 at 12 51 26 Screenshot 2024-07-08 at 10 40 22

@ciampo ciampo self-assigned this Jul 8, 2024
@ciampo ciampo added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Jul 8, 2024
@ciampo ciampo marked this pull request as ready for review July 8, 2024 08:49
@ciampo ciampo requested a review from ajitbohra as a code owner July 8, 2024 08:49
Copy link

github-actions bot commented Jul 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: ciampo <mciampini@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>

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

@ciampo ciampo requested review from jameskoster and a team July 8, 2024 08:49
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Tests as advertised and code looks good 👍 🚀

I'm starting to prefer this solution design-wise, too.

packages/components/src/custom-select-control-v2/styles.ts Outdated Show resolved Hide resolved
@ciampo ciampo force-pushed the fix/custom-select-control-v2-legacy-adapter-checkmark-vertical-alignment branch from e36d376 to 43cb26f Compare July 8, 2024 10:48
@ciampo ciampo enabled auto-merge (squash) July 8, 2024 10:49
Copy link

github-actions bot commented Jul 8, 2024

Flaky tests detected in 43cb26f.
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/9838189617
📝 Reported issues:

@ciampo ciampo merged commit 9d43af4 into trunk Jul 8, 2024
61 checks passed
@ciampo ciampo deleted the fix/custom-select-control-v2-legacy-adapter-checkmark-vertical-alignment branch July 8, 2024 11:23
@github-actions github-actions bot added this to the Gutenberg 18.8 milestone Jul 8, 2024
huubl pushed a commit to huubl/gutenberg that referenced this pull request Jul 10, 2024
* CustomSelectControlV2: keep item checkmark top aligned

* CHANGELOG

---

Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
carstingaxion pushed a commit to carstingaxion/gutenberg that referenced this pull request Jul 18, 2024
* CustomSelectControlV2: keep item checkmark top aligned

* CHANGELOG

---

Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants