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

[EuiButtonGroup] Better differentiate the current selection #6819

Closed
ryankeairns opened this issue May 30, 2023 · 9 comments
Closed

[EuiButtonGroup] Better differentiate the current selection #6819

ryankeairns opened this issue May 30, 2023 · 9 comments
Labels
design decision feature request low hanging fruit An issue, often a bug, that is lower effort and clearly ought to be fixed

Comments

@ryankeairns
Copy link
Contributor

ryankeairns commented May 30, 2023

Is your feature request related to a problem? Please describe.
[internal feedback] I cannot tell with confidence which value is actually selected by looking at this unless I click and make the selection. But I just want to know what's selected, not change it.

Describe the solution you'd like
Can I suggest to make the selected item have more differentiation, perhaps an outline in addition to darker colouring. (this is one example to highlight).

Proposed redesign

Screen Shot 2023-06-22 at 8 53 03 AM

Additional context
This is particularly problematic when only two options are presented (see below):

image (20)

For reference, we apply margin around the currently active option in the compressed version of this input. Perhaps we can apply a similar style to the default size EuiButtonGroup, as well.

Screen Shot 2023-05-26 at 7 18 37 AM

@cee-chen cee-chen added the low hanging fruit An issue, often a bug, that is lower effort and clearly ought to be fixed label Jun 12, 2023
@cee-chen
Copy link
Contributor

cee-chen commented Jun 12, 2023

@ryankeairns any suggestions or opinions on what the solution/fix should be? Should normal be way more bold than low? Should we, e.g. change the highlight color to primary instead? This isn't an option, as EuiButtonGroup supports multiple color types.

Edit for clarity - I'm looking for a clear acceptance criteria to close out this issue :)

@ryankeairns
Copy link
Contributor Author

ryankeairns commented Jun 22, 2023

I have not sketched any design alternatives, so I'm going on quick thoughts:

  1. Yes, additional font weight (i.e. bold) would help, and
  2. I'm inclined to copy the compressed version where there is padding within the group (as seen above)

Caveat:
We'll likely want to retain a height of 40px so that this matches other form controls
Edit: see Cee's comment below

Screen Shot 2023-06-22 at 8 53 03 AM

@cee-chen
Copy link
Contributor

cee-chen commented Jun 22, 2023

Good news on 1., I've increased the font weight of selected buttons as part of the component's Emotion conversion (#6841).

I personally like the look of 2 a lot!

We'll likely want to retain a height of 40px so that this matches other form controls

I don't think this is correct. The button group should match the height of the buttons at the specified sizes, the compressed styling is what matches the form control heights.

@ryankeairns
Copy link
Contributor Author

Added the proposed design from my previous comment to the issue description.
Seems solid enough to proceed that direction.

@cee-chen
Copy link
Contributor

I spiked this out a bit but I'm running into slight difficulties.

  1. There's a very light border between multiple buttons of the same color/selection that isn't present on compressed. Should we remove this border entirely going forward? Does removing the border visual difficulty between delineating between options?
  2. Multiple selections on the compressed style do not combine/join together visually in compressed styling but they previous did in uncompressed styling. Is this fine/an intentional choice?

Uncompressed:

Compressed

@cee-chen
Copy link
Contributor

cee-chen commented Jun 23, 2023

I spiked this out because it's easier to have something to look at in-browser: #6876 / https://eui.elastic.co/pr_6876/#/navigation/button#button-groups (make sure to test selecting all and unselecting all on the 2nd example)

TBH, I'm not sure I'm a huge fan of the change in reality. The styling change is significantly more complex than I thought (especially for disabled states, particularly when a button inside a group is disabled but the entire group is not disabled), and to be honest, visually I can barely even see the 2px border especially on certain color combos.

I think the font-weight change I've already made is already a sufficient indicator for selection, and it might be worth considering skipping the pill/padding changing. Thoughts?

@ryankeairns
Copy link
Contributor Author

The styling change is significantly more complex than I thought (especially for disabled states, particularly when a button inside a group is disabled but the entire group is not disabled)

Helpful context; not surprising to learn :)

I think the font-weight change I've already made is already a sufficient indicator for selection, and it might be worth considering skipping the pill/padding changing.

In light of the additional context - and that this is most relevant in situations with exactly two options - this seems like the practical thing to do first. We can then keep an eye/ear open for additional feedback.

@cee-chen
Copy link
Contributor

Thanks Ryan!

In light of the additional context - and that this is most relevant in situations with exactly two options - this seems like the practical thing to do first. We can then keep an eye/ear open for additional feedback.

For the purposes of this open issue, should we go ahead and close it for now until we hear that more change is needed? Or we could leave the issue open and let it auto-close in a year 🤷

@ryankeairns
Copy link
Contributor Author

Let's close it. People can still add comments / provide feedback but, and until this happens, it won't land on our roadmap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design decision feature request low hanging fruit An issue, often a bug, that is lower effort and clearly ought to be fixed
Projects
None yet
Development

No branches or pull requests

2 participants