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

Fixed EuiSwitch clicking on disabled label #2575

Merged
merged 3 commits into from
Dec 3, 2019

Conversation

sulemanof
Copy link
Contributor

Summary

This fixes the regression of EuiSwitch component:

  • The switch is clickable even in disabled state if click on label:

switch_disabled

  • Label displays under the switch button if there is not enough space:

image

Found in kibana:

switch

Using flex box it looks better:

image

Checklist

  • Checked in dark mode
  • Checked in mobile
  • Checked in IE11 and Firefox
  • Props have proper autodocs
  • Added documentation 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

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Thanks for catching and fixing this, @sulemanof

@@ -1,6 +1,6 @@
.euiSwitch {
position: relative;
display: inline-block;
display: flex;
Copy link
Contributor

@thompsongl thompsongl Dec 2, 2019

Choose a reason for hiding this comment

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

This should remain as inline-block. The display property here hasn't changed recently, and there doesn't appear to be any regressions with layout.

The .euiSwitch__label display property should be inline to match previous word wrapping.

Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed there's an issue using the <p> tag when placed inside of an EuiText block and have already started a PR to fix this. I'll fix the wrapping as well in that PR, so we can safely, for now, revert this display change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well. The display change was reverted

@sulemanof sulemanof requested a review from thompsongl December 3, 2019 14:28
Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Thanks, @sulemanof!

@cchaos has text wrapping adjustments in #2585 that should address the same concerns you had with layout

@sulemanof sulemanof merged commit 4d646fa into elastic:master Dec 3, 2019
@sulemanof sulemanof deleted the fix_switch branch December 3, 2019 14:54
thompsongl added a commit that referenced this pull request Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants