[EuiButtonGroup] Improve interaction states by elizabetdev · Pull Request #4142 · elastic/eui

elizabetdev · 2020-10-15T17:43:08Z

Summary

Closes #4105.

This PR improves the EuiButtonGroup interaction states.

Checklist

Check against all themes for compatibility in both light and dark modes
[ ] Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
[ ] Props have proper autodocs
[ ] Added documentation
~~[ ] Checked Code Sandbox works for the any docs examples~~
[ ] Added or updated jest tests
~~[ ] Checked for breaking changes and labeled appropriately~~
Checked for accessibility including keyboard-only and screenreader modes
A changelog entry exists and is marked appropriately

…true

…mpressed-a11y-colors

kibanamachine · 2020-10-15T18:08:04Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4142/

cchaos · 2020-10-15T18:15:18Z

Unfortunately this going to have a pretty big conflict with #4056 which is why it's been put into draft mode. We'll want to wait til that PR gets merged first as it completely re-works the styles because of the updated component structure.

…mpressed-a11y-colors

kibanamachine · 2020-10-19T17:02:29Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4142/

kibanamachine · 2020-10-19T17:44:15Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4142/

…mpressed-a11y-colors

kibanamachine · 2020-10-19T18:26:11Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4142/

cchaos

Thanks for tackling this one. Sorry for the conflicts that surely ensued. I'm visually ok with more pronounced selected state, but it did create a few visual regressions.

src/components/button/button_group/_button_group_button.scss

kibanamachine · 2020-10-20T18:42:51Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4142/

kibanamachine · 2020-10-20T20:07:23Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4142/

…mpressed-a11y-colors

elizabetdev · 2020-10-21T12:14:44Z

@cchaos thanks for your previous feedback.

I decided to make all the interaction states for the EuiButtonGroup similar. The only difference is that the compressed buttons have a focus ring.

The best way to test the changes is to replace the files src-docs/src/views/button/button_group.js and src-docs/src/views/form_compressed/complex_example.js with https://gist.github.com/miukimiu/13064cd5fa1345ff39a8569a0ceddfe8.

Then in /forms/compressed-forms#complex-example you can test the compressed EuiButtonGroup versions. And in /navigation/button#button-groups you can test the non-compressed versions.

kibanamachine · 2020-10-21T12:41:58Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4142/

cchaos

👍 LGTM. Thanks for those gists for testing!

The only possibly one thing you might want to tweak is the hover state affecting the text color. It is causing the text color to change in some instances like the Warning and Success colors, and the Text color in dark mode.

cchaos · 2020-10-21T15:04:36Z

src/components/button/button_group/_button_group_button.scss

+ * Medium and Small sizing (regular button style)
+ */
+
+// sass-lint:disable nesting-depth


I'd guess this lint disable is not needed anymore?

elizabetdev · 2020-10-21T18:05:02Z

@cchaos the .is-selected hover states affecting the texts were on purpose.

I thought it didn't have enough contrast when the background gets darker:

So I decided to use the chooseLightOrDarkText() based on the contrast:

Is it too much? 😛

kibanamachine · 2020-10-21T18:30:47Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4142/

cchaos · 2020-10-21T18:57:25Z

I'm thinking that in general the buttons are going too dark on hover. We should probably be consistent with our base buttons which only darken by 5% in default theme, which we should use in Amsterdam too.

…mpressed-a11y-colors

kibanamachine · 2020-10-22T12:28:34Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4142/

monfera · 2020-10-26T12:50:02Z

Thanks a lot @miukimiu @cchaos @snide, looks great!

elizabetdev added 7 commits October 15, 2020 14:33

Improved EuiButtonGroup focus and selected states when compressed is …

38e121c

…true

Fixing amsterdam

a9467e6

disabled and selected

d027e6f

Amsterdam theme

40ba2c1

More changes

4c6a8e0

Merge remote-tracking branch 'upstream/master' into EuiButtonGroup-co…

242465d

…mpressed-a11y-colors

CL

cb473d7

elizabetdev marked this pull request as draft October 15, 2020 17:52

elizabetdev added 3 commits October 19, 2020 14:43

Merge remote-tracking branch 'upstream/master' into EuiButtonGroup-co…

464aae1

…mpressed-a11y-colors

New structure compressed styles

dee9a6c

Merge remote-tracking branch 'upstream/master' into EuiButtonGroup-co…

33ae958

…mpressed-a11y-colors

Small details

1846d38

elizabetdev added 2 commits October 19, 2020 18:56

Merge remote-tracking branch 'upstream/master' into EuiButtonGroup-co…

d5052f5

…mpressed-a11y-colors

Highlight bg

c0a2b25

elizabetdev marked this pull request as ready for review October 19, 2020 18:05

elizabetdev requested a review from cchaos October 19, 2020 18:05

cchaos suggested changes Oct 19, 2020

View reviewed changes

src/components/button/button_group/_button_group_button.scss Show resolved Hide resolved

src/components/button/button_group/_button_group_button.scss Outdated Show resolved Hide resolved

Making all button group states to behave similarly

4e71b03

Removing unnecessary styles

a141bd8

elizabetdev added 2 commits October 21, 2020 12:39

Merge remote-tracking branch 'upstream/master' into EuiButtonGroup-co…

085b703

…mpressed-a11y-colors

CL

212203c

elizabetdev changed the title ~~[EuiButtonGroup] Improve focus, selected and disabled states when compressed~~ [EuiButtonGroup] Improve interaction states Oct 21, 2020

Adding comment

ddd3a4d

elizabetdev added 2 commits October 21, 2020 13:17

Fixing cl issue

ff7f926

Adding comment

73ee0d3

elizabetdev requested a review from cchaos October 21, 2020 13:41

cchaos approved these changes Oct 21, 2020

View reviewed changes

removing unecessary sass-lint comment

6317558

elizabetdev added 2 commits October 22, 2020 12:58

More consistent darken and transparentize values

252de08

Merge remote-tracking branch 'upstream/master' into EuiButtonGroup-co…

ec7a2a4

…mpressed-a11y-colors

elizabetdev merged commit a61138c into elastic:master Oct 22, 2020

thompsongl mentioned this pull request Oct 27, 2020

Upgrade EUI to v30.1.1 elastic/kibana#81499

Merged

cjcenizal mentioned this pull request Apr 2, 2021

[EuiButtonGroup] Usability/accessibility enhancements #4684

Closed

Conversation

elizabetdev commented Oct 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

kibanamachine commented Oct 15, 2020

Uh oh!

cchaos commented Oct 15, 2020

Uh oh!

kibanamachine commented Oct 19, 2020

Uh oh!

kibanamachine commented Oct 19, 2020

Uh oh!

kibanamachine commented Oct 19, 2020

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Oct 20, 2020

Uh oh!

kibanamachine commented Oct 20, 2020

Uh oh!

elizabetdev commented Oct 21, 2020

Uh oh!

kibanamachine commented Oct 21, 2020

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

cchaos Oct 21, 2020

Choose a reason for hiding this comment

Uh oh!

elizabetdev commented Oct 21, 2020

Uh oh!

kibanamachine commented Oct 21, 2020

Uh oh!

cchaos commented Oct 21, 2020

Uh oh!

kibanamachine commented Oct 22, 2020

Uh oh!

monfera commented Oct 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

elizabetdev commented Oct 15, 2020 •

edited

Loading