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.

Sign up for GitHub

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

Jump to bottom

Add event target checker for EuiOverlayMask's onClick prop #3462

Merged
merged 10 commits into from
May 14, 2020
Merged

Add event target checker for EuiOverlayMask's onClick prop #3462

merged 10 commits into from
May 14, 2020

Conversation

TAYTS
Copy link
Contributor

@TAYTS TAYTS commented May 12, 2020 •
edited
Loading

Summary

Added event target checker for EuiOverlayMask's onClick prop to prevent the onClick function from firing due to the wrapping child elements's onClick event propagation.
Issue: #3429

Checklist

  • Check against all themes for compatibility in both light and dark modes~~
  • 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

@kibanamachine
Copy link

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?

@TAYTS TAYTS mentioned this pull request May 12, 2020
Closed
@cchaos
Copy link
Contributor

cchaos commented May 12, 2020

J, test this

@kibanamachine
Copy link

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

cchaos
cchaos suggested changes May 12, 2020
View reviewed changes
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Thanks @TAYTS ! This works really well in the overlay mask docs section where the text is now selectable but can still click the mask part to close.

Screen Shot 2020-05-12 at 11 27 35 AM

I just don't think we should add it to all the modal examples (reasons below).

src-docs/src/views/modal/confirm_modal.js Outdated Show resolved Hide resolved
src-docs/src/views/modal/modal.js Outdated Show resolved Hide resolved
@TAYTS TAYTS force-pushed the feature/add-element-checker-for-overlay-mask branch from 42b5ada to 1562383 Compare May 12, 2020 15:38
@cchaos
Copy link
Contributor

cchaos commented May 12, 2020

retest

@kibanamachine
Copy link

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

@cchaos
Adding back and to the overflow example
a744698 
Changed the buttons to be a simple "Close" button so that it becomes a more simple modal example that is ok to work with the onClose of the overflow mask
@cchaos
Forgot to add the onClick
10861e2
@cchaos
Merge branch 'master' into feature/add-element-checker-for-overlay-mask
a1024a6
@cchaos
Copy link
Contributor

cchaos commented May 12, 2020

retest

@kibanamachine
Copy link

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

@TAYTS TAYTS requested a review from cchaos May 13, 2020 01:45
@cchaos
Copy link
Contributor

cchaos commented May 13, 2020

retest

@kibanamachine
Copy link

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

@cchaos
Copy link
Contributor

cchaos commented May 13, 2020

retest

@kibanamachine
Copy link

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

@chandlerprall
Copy link
Contributor

CI is failing with

20:41:21 /app/src-docs/src/views/modal/overflow_test.js
20:41:21   5:3  error  'EuiButtonEmpty' is defined but never used  @typescript-eslint/no-unused-vars
20:41:21 
20:41:21 ✖ 1 problem (1 error, 0 warnings)

@cchaos
Copy link
Contributor

cchaos commented May 14, 2020

Thanks on me, thanks @chandlerprall

@cchaos
Copy link
Contributor

cchaos commented May 14, 2020

retest

@kibanamachine
Copy link

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

cchaos
cchaos approved these changes May 14, 2020
View reviewed changes
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

🎉 Yay, we're passing! Thanks again @TAYTS

@cchaos cchaos merged commit 412e7aa into elastic:master May 14, 2020
@TAYTS TAYTS deleted the feature/add-element-checker-for-overlay-mask branch May 15, 2020 04:30
@thompsongl thompsongl mentioned this pull request Jun 3, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cchaos cchaos cchaos approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@TAYTS @kibanamachine @cchaos @chandlerprall