Skip to content

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

Merged
cchaos merged 10 commits intoelastic:masterfrom
TAYTS:feature/add-element-checker-for-overlay-mask
May 14, 2020
Merged

Add event target checker for EuiOverlayMask's onClick prop#3462
cchaos merged 10 commits intoelastic:masterfrom
TAYTS:feature/add-element-checker-for-overlay-mask

Conversation

@TAYTS
Copy link
Copy Markdown
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
Copy Markdown

kibanamachine commented May 12, 2020

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
Copy Markdown
Contributor

cchaos commented May 12, 2020

J, test this

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented May 12, 2020

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

cchaos
cchaos suggested changes May 12, 2020
View reviewed changes
Copy link
Copy Markdown
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).

@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
Copy Markdown
Contributor

cchaos commented May 12, 2020

retest

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented May 12, 2020

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

cchaos added 3 commits May 12, 2020 17:33
@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
Copy Markdown
Contributor

cchaos commented May 12, 2020

retest

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented May 12, 2020

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
Copy Markdown
Contributor

cchaos commented May 13, 2020

retest

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented May 13, 2020

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

@cchaos
Copy link
Copy Markdown
Contributor

cchaos commented May 13, 2020

retest

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented May 13, 2020

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

@chandlerprall
Copy link
Copy Markdown
Contributor

chandlerprall commented May 14, 2020

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
Copy Markdown
Contributor

cchaos commented May 14, 2020

Thanks on me, thanks @chandlerprall

@cchaos
Copy link
Copy Markdown
Contributor

cchaos commented May 14, 2020

retest

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented May 14, 2020

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
Copy Markdown
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

1 more reviewer

@cchaos cchaos cchaos approved these changes

Reviewers whose approvals may not affect merge requirements

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