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

Interactive IssueLabelToken does not render any hover effects #3072

Closed
iansan5653 opened this issue Mar 23, 2023 · 7 comments · Fixed by #3112
Closed

Interactive IssueLabelToken does not render any hover effects #3072

iansan5653 opened this issue Mar 23, 2023 · 7 comments · Fixed by #3112
Assignees
Labels
bug Something isn't working react size: pebble

Comments

@iansan5653
Copy link
Contributor

Description

The Token component gets a darker background and a box shadow when it is interactive and hovered over. The IssueLabelToken is a variant of this component that should, to the extent possible, match the Token component and work well with it. However, IssueLabelToken does not show any hover states when it is interactive, making it look broken next to Token.

I can see why it would be difficult to show a different background color on hover because the label has a hardcoded user-controlled background color, but we should at least show the box shadow on hover to make it match the Token.

🎬 Video alt text: A screen capture showing a mouse pointer hovering over items in a set of tokens. Each token shows a mild shadow and a darker background on hover except for the tokens representing labels, which only show indicate interactivity by showing a 'click' mouse pointer.

Screen.Recording.2023-03-23.at.3.52.36.PM.mov

Steps to reproduce

Render an interactive IssueLabelToken, ie by adding as="button" or as="a":

<IssueLabelToken text="good first issue" fillColor="#0366d6" as="button" />

Version

v35.22.0

Browser

Chrome

@iansan5653 iansan5653 added the bug Something isn't working label Mar 23, 2023
@tallys
Copy link

tallys commented Apr 3, 2023

tagging in @lukasoppermann for visibility

@langermank
Copy link
Contributor

I created a quick prototype for this one as a way for me to experiment with the new color-mix() CSS function 😄 #3112

I think it needs fine tuning around hue and saturation, as it can easily look muddy by just adding white or black. Unassigning myself for now– maybe @lukasoppermann and I can pair on it a bit.

@langermank langermank removed their assignment Apr 3, 2023
@lesliecdubs
Copy link
Member

👋🏻 @lukasoppermann I'm assigning this your way based on the discussion above, but please give me or @tallys a shout if you're not sure how to fit this in.

@lukasoppermann
Copy link
Contributor

@iansan5653 where do you need a hover for the IssueLabelToken?

I think 🤔 this specific component has historically not been interactive. It is more a label that is visible but not one that can be clicked.

@iansan5653
Copy link
Contributor Author

iansan5653 commented Apr 11, 2023

where do you need a hover for the IssueLabelToken?

In project board cards, the tokens on the cards can be clicked to filter the view. For example, clicking the bug label would filter the view by label:bug.

I think 🤔 this specific component has historically not been interactive.

The regular Token variants do render hover effects and the IssueLabelToken inherits the as="button" onClick={...} props -- to me that feels like it should imply that it supports interactivity. If we really don't want it to be interactive from a design perspective, we should remove those props on the component.

It is more a label that is visible but not one that can be clicked.

I actually can't think of any places on GitHub where we render labels that don't respond to clicks. Almost everywhere, clicking a label navigates to the issues index page for that label.

@lukasoppermann
Copy link
Contributor

lukasoppermann commented Apr 11, 2023

In project board cards, the tokens on the cards can be clicked to filter the view. For example, clicking the bug label would filter the view by label:bug.

Okay, yeah, that makes sense. I'll look into @langermank work. However this only covers Primer React, would that be enough?

@iansan5653
Copy link
Contributor Author

iansan5653 commented Apr 11, 2023

However this only covers Primer React, would that be enough?

👍 I only work in React apps so I can't speak for Rails/VC but I think it's fine for now. React apps do tend to be more interactive/dynamic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working react size: pebble
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants