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

Re-visit hover state for primary button (e.g. currently there is no visual indicator other than a cursor change) #1271

Closed
Tracked by #1851
halocline opened this issue Nov 18, 2020 · 10 comments
Assignees
Labels
Owner:Design Used in issues that are being worked on/should be worked on by a designer.

Comments

@halocline
Copy link
Collaborator

No description provided.

@halocline halocline added Owner:Design Used in issues that are being worked on/should be worked on by a designer. Ideate labels Nov 18, 2020
@Keeth-HPe
Copy link

Keeth-HPe commented Nov 24, 2021

@ericsoderberghp
Copy link
Contributor

I think the elevation approach aligns better with how we handle hover styling for Card. The border treatment doesn't seem as consistent with anything else we have in the design system currently.

A couple of other ideas:

  • slight change of tint, perhaps just adding a little opacity?
  • drastic change of background, perhaps even aligning with the coloring for default buttons, with background-contrast and text-strong?

@Keeth-HPe
Copy link

Keeth-HPe commented Nov 29, 2021

@ericsoderberghp Thank you so much for the feedback, I tried the options you have mentioned,

  1. Adding opacity: I'm afraid it might make the primary button look under underemphasized while hovering. I'm leaning towards making it look highlighted. However, I've added this option too ( the change is visible only if the opacity is/ below 60%)

  2. Background contrast + text strong: In cases where more than 2 or 3 buttons are being used, I feel this styling will conflict with secondary and Default buttons, causing confusion to the user. So I've tried using the brand color instead, which is a drastic change of BG.
    Please do take a look :)

Prototype link: https://www.figma.com/proto/Oi5UEEQ33VCAyOKQcZ8Nr8/HPE-Button-Component?page-id=2282%3A5045&node-id=4243%3A9054&viewport=295%2C48%2C0.26&scaling=min-zoom&starting-point-node-id=4243%3A9054

@bahriaditi @vavalos5

@ericsoderberghp
Copy link
Contributor

Nice to see more options.

  • I think the light and dark modes aren't aligned? Option 3 light has 60% opacity but that is Option 2 dark.
  • Option 2 seems like it might not be accessible, with the dark text on the darker background.
  • Option 3 (60% background opacity) is subtle but seems viable to me.

I don't think we have to make the hover styling even more prominent than the primary button already is, in case that opens up some more possibilities.

@bahriaditi
Copy link
Contributor

Hi @Keeth-HPe

Thanks for providing all of these options! I think we should stick to a fairly straightforward approach here.
I would suggest a color change, making the background a bit darker in both light and dark modes (see attached).

I checked for accessibility and these colors pass the recommended color contrast ratios. In addition to the primary green button, I also went ahead and suggested hover states for the Color buttons (blue and purple).

Let me know if you agree with this approach:

Screen Shot 2021-11-30 at 10 58 34 AM

Figma: https://www.figma.com/file/Oi5UEEQ33VCAyOKQcZ8Nr8/HPE-Button-Component?node-id=4401%3A9254

@Keeth-HPe
Copy link

I agree with this approach 👍 @bahriaditi

@vavalos5
Copy link
Collaborator

vavalos5 commented Dec 2, 2021

The problem with making the background colors our dark colors are that the text is now not legible. After running some accessibility tests, it looks like the light mode and dark mode for the primary button do not pass accessibility. We might want to look at other ways of showing hover for our primary color. Like perhaps we need to add a new tint for light mode and a tint for dark mode. Please read my other comments on the Figma file

I as taking a peek at Carbon's Design System color palette and they have various of contrast. Now this is due to the fact that they offer other background colors in their theme. While we don't need to extend our color pallet to this extent, we should consider other tints and hues for our primary button on hover. Hope this helps.

@bahriaditi
Copy link
Contributor

@vavalos5 great eye, as always! I missed checking for text legibility. @Keeth-HPe can you explore some more color options based on Vicky's comments please? I'll move this ticket back to "in progress"

@bahriaditi
Copy link
Contributor

@vavalos5 great eye, as always! I missed checking for text legibility. @Keeth-HPe can you explore some more color options based on Vicky's comments please? I'll move this ticket back to "in progress"

@Keeth-HPe Spoke to soon! Checked in with Vicky, and these colors do actually pass the WCAG AA guidelines based on results here: https://webaim.org/resources/contrastchecker/

Note that we are making one trade off with this approach:

  • On dark mode, the primary button hover state background color does not pass the required contrast ratio. But in my opinion, it is more important that the text label is legible (which it is).

@taysea
Copy link
Collaborator

taysea commented Aug 30, 2023

Closing this, in v5 of the theme we added a hover gradient.

@taysea taysea closed this as completed Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Owner:Design Used in issues that are being worked on/should be worked on by a designer.
Projects
None yet
Development

No branches or pull requests

6 participants