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.

Sign up for GitHub

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

Jump to bottom

Use default cursor in CustomSelectControl items #20217

Merged
merged 1 commit into from
Nov 28, 2020
Merged

Use default cursor in CustomSelectControl items #20217

merged 1 commit into from
Nov 28, 2020

Conversation

Aljullu
Copy link
Contributor

@Aljullu Aljullu commented Feb 13, 2020

Description

While reviewing woocommerce/woocommerce-blocks#1727, @mikejolley noticed the cursor changes from default to text when hovering the text of CustomSelectControl items. This PR makes the cursor consistent so the default cursor is used in the entire item.

This way we match the native <select> element.

How has this been tested?

  • Testing the CustomSelectControl in the story board.
  • Verifying there were no regressions in the editor font size select.

Screenshots

Before After
Screenshoft from before Screenshoft from before

Types of changes

CSS enhancement.

@Aljullu Aljullu requested a review from epiqueras February 13, 2020 15:35
@Aljullu Aljullu self-assigned this Feb 13, 2020
@Aljullu Aljullu mentioned this pull request Feb 13, 2020
Merged
@epiqueras epiqueras added the Needs Accessibility Feedback Need input from accessibility label Feb 13, 2020
@epiqueras
Copy link
Contributor

Pinged a11y, we should also see if this should be added to Downshift, cc @silviuavram.

@youknowriad
Copy link
Contributor

Seems like it should be a "pointer" cursor for me

@Aljullu
Copy link
Contributor Author

Aljullu commented Feb 14, 2020

@youknowriad I went with cursor: default because that's what's used for native selects in all platforms I checked (Windows, macOS and Linux). cursor: pointer is usually reserved for links, while buttons, selects and most input types use the default cursor.

gziolo
gziolo approved these changes Nov 28, 2020
View reviewed changes
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Let’s get it in.

@gziolo gziolo added [Package] Components /packages/components and removed Needs Accessibility Feedback Need input from accessibility labels Nov 28, 2020
@gziolo gziolo merged commit 5417b91 into master Nov 28, 2020
@gziolo gziolo deleted the fix/custom-select-control-default-cursor branch November 28, 2020 07:22
@github-actions github-actions bot added this to the Gutenberg 9.5 milestone Nov 28, 2020
@gziolo
Copy link
Member

gziolo commented Nov 28, 2020

I should check the latest version of the file, the same line is twice there, reverting 😅

gziolo added a commit that referenced this pull request Nov 28, 2020
@gziolo
Revert "Use default cursor in CustomSelectControl items (#20217)"
32dca44 
This reverts commit 5417b91.
@gziolo gziolo mentioned this pull request Nov 28, 2020
Merged
gziolo added a commit that referenced this pull request Nov 28, 2020
@gziolo
Revert "Use default cursor in CustomSelectControl items (#20217)" (#2…
8537da7 
…7349)

This reverts commit 5417b91.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gziolo gziolo gziolo approved these changes

@epiqueras epiqueras Awaiting requested review from epiqueras

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@chrisvanpatten chrisvanpatten Awaiting requested review from chrisvanpatten

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

@youknowriad youknowriad Awaiting requested review from youknowriad

Assignees

@Aljullu Aljullu

Labels
[Package] Components /packages/components
Projects
None yet
Milestone
Gutenberg 9.5
Development

Successfully merging this pull request may close these issues.
4 participants
@Aljullu @epiqueras @youknowriad @gziolo