-
Notifications
You must be signed in to change notification settings - Fork 861
[EuiModal] Support closing with outside click #9137
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
Conversation
via new `outsideClickCloses` prop
| cy.get('[data-test-subj="modal-trigger"]').click(); | ||
| cy.get('div.euiModal').should('exist'); | ||
| cy.get('div.euiOverlayMask').should('exist'); | ||
| cy.get('div.euiOverlayMask').click({ force: true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 since the overlay is a plain div, I believe click({ force: true }) is appropriate
💚 Build SucceededHistory
cc @acstll |
💚 Build Succeeded
History
cc @acstll |
tkajtoch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for addressing my comment. The changes look great!
If there's ever need to distinguish between these two onClose triggers, we could just add a second argument with a reason or a source
- `@elastic/eui`: `v107.0.1` ⏩ `v108.0.0` - `@elastic/eui-theme-borealis`: `v3.6.0` ⏩ `v4.0.0` --- ## Changes >[!IMPORTANT] This PR removes code related to the legacy Amsterdam theme. But it purposefully keeps Amsterdam palettes in `kbn-palettes` as those are standalone. - removed code related to the legacy theme Amsterdam [[#9090](elastic/eui#9090)] - updated custom types to align with changes to the event type of `onClose` on `EuiModal` [[#9137](elastic/eui#9137)] ## Package updates ### [`v108.0.0`](https://github.com/elastic/eui/releases/v108.0.0) - Updated `EuiModal` to support closing on outside click, via the new `outsideClickCloses` prop ([#9137](elastic/eui#9137)) **Breaking changes** - Removed all "Amsterdam" theme related code in `src/themes/amsterdam` - EUI now only supports the "Borealis" theme in `eui-theme-borealis` ([#9090](elastic/eui#9090)) - Removed `euiTheme.flags.hasGlobalFocusColor` ([#9090](elastic/eui#9090)) - Removed `euiTheme.flags.hasVisColorAdjustment` ([#9090](elastic/eui#9090)) - Removed `hasVisColorAdjustment` argument from color palettes (used in `euiPaletteColorBlindBehindText`, `euiPaletteForTemperature`, `euiPaletteComplementary`, `euiPaletteCool`) ([#9090](elastic/eui#9090)) - Removed `euiTheme.flags.buttonVariant` and `euiTheme.flags.formVariant` ([#9090](elastic/eui#9090)) - Removed `euiTheme.components.keyPadMenuItemBackgroundDisabledSelect` ([#9090](elastic/eui#9090)) - Removed legacy SCSS files from `src/global_styling/variables`, `src/global_styling/mixins` and `src/global_styling/functions` - if needed, use them from `eui-theme-common` instead ([#9090](elastic/eui#9090)) ### [`v4.0.0`](https://github.com/elastic/eui/releases/v4.0.0) **Breaking changes** - Removed `euiTheme.flags.hasGlobalFocusColor` ([#9090](elastic/eui#9090)) - Removed `euiTheme.flags.hasVisColorAdjustment` ([#9090](elastic/eui#9090)) - Removed `euiTheme.flags.buttonVariant` and `euiTheme.flags.formVariant` ([#9090](elastic/eui#9090)) - Removed `euiTheme.components.keyPadMenuItemBackgroundDisabledSelect` ([#9090](elastic/eui#9090)) --------- Co-authored-by: Elastic Machine <[email protected]>
- `@elastic/eui`: `v107.0.1` ⏩ `v108.0.0` - `@elastic/eui-theme-borealis`: `v3.6.0` ⏩ `v4.0.0` --- ## Changes >[!IMPORTANT] This PR removes code related to the legacy Amsterdam theme. But it purposefully keeps Amsterdam palettes in `kbn-palettes` as those are standalone. - removed code related to the legacy theme Amsterdam [[elastic#9090](elastic/eui#9090)] - updated custom types to align with changes to the event type of `onClose` on `EuiModal` [[elastic#9137](elastic/eui#9137)] ## Package updates ### [`v108.0.0`](https://github.com/elastic/eui/releases/v108.0.0) - Updated `EuiModal` to support closing on outside click, via the new `outsideClickCloses` prop ([elastic#9137](elastic/eui#9137)) **Breaking changes** - Removed all "Amsterdam" theme related code in `src/themes/amsterdam` - EUI now only supports the "Borealis" theme in `eui-theme-borealis` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.flags.hasGlobalFocusColor` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.flags.hasVisColorAdjustment` ([elastic#9090](elastic/eui#9090)) - Removed `hasVisColorAdjustment` argument from color palettes (used in `euiPaletteColorBlindBehindText`, `euiPaletteForTemperature`, `euiPaletteComplementary`, `euiPaletteCool`) ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.flags.buttonVariant` and `euiTheme.flags.formVariant` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.components.keyPadMenuItemBackgroundDisabledSelect` ([elastic#9090](elastic/eui#9090)) - Removed legacy SCSS files from `src/global_styling/variables`, `src/global_styling/mixins` and `src/global_styling/functions` - if needed, use them from `eui-theme-common` instead ([elastic#9090](elastic/eui#9090)) ### [`v4.0.0`](https://github.com/elastic/eui/releases/v4.0.0) **Breaking changes** - Removed `euiTheme.flags.hasGlobalFocusColor` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.flags.hasVisColorAdjustment` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.flags.buttonVariant` and `euiTheme.flags.formVariant` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.components.keyPadMenuItemBackgroundDisabledSelect` ([elastic#9090](elastic/eui#9090)) --------- Co-authored-by: Elastic Machine <[email protected]>
- `@elastic/eui`: `v107.0.1` ⏩ `v108.0.0` - `@elastic/eui-theme-borealis`: `v3.6.0` ⏩ `v4.0.0` --- ## Changes >[!IMPORTANT] This PR removes code related to the legacy Amsterdam theme. But it purposefully keeps Amsterdam palettes in `kbn-palettes` as those are standalone. - removed code related to the legacy theme Amsterdam [[elastic#9090](elastic/eui#9090)] - updated custom types to align with changes to the event type of `onClose` on `EuiModal` [[elastic#9137](elastic/eui#9137)] ## Package updates ### [`v108.0.0`](https://github.com/elastic/eui/releases/v108.0.0) - Updated `EuiModal` to support closing on outside click, via the new `outsideClickCloses` prop ([elastic#9137](elastic/eui#9137)) **Breaking changes** - Removed all "Amsterdam" theme related code in `src/themes/amsterdam` - EUI now only supports the "Borealis" theme in `eui-theme-borealis` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.flags.hasGlobalFocusColor` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.flags.hasVisColorAdjustment` ([elastic#9090](elastic/eui#9090)) - Removed `hasVisColorAdjustment` argument from color palettes (used in `euiPaletteColorBlindBehindText`, `euiPaletteForTemperature`, `euiPaletteComplementary`, `euiPaletteCool`) ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.flags.buttonVariant` and `euiTheme.flags.formVariant` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.components.keyPadMenuItemBackgroundDisabledSelect` ([elastic#9090](elastic/eui#9090)) - Removed legacy SCSS files from `src/global_styling/variables`, `src/global_styling/mixins` and `src/global_styling/functions` - if needed, use them from `eui-theme-common` instead ([elastic#9090](elastic/eui#9090)) ### [`v4.0.0`](https://github.com/elastic/eui/releases/v4.0.0) **Breaking changes** - Removed `euiTheme.flags.hasGlobalFocusColor` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.flags.hasVisColorAdjustment` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.flags.buttonVariant` and `euiTheme.flags.formVariant` ([elastic#9090](elastic/eui#9090)) - Removed `euiTheme.components.keyPadMenuItemBackgroundDisabledSelect` ([elastic#9090](elastic/eui#9090)) --------- Co-authored-by: Elastic Machine <[email protected]>
Summary
Resolves #8967
This adds a new
outsideClickClosesprop toEuiModal. Whentrue, the modal will close when the overlay mask (outside) is clicked.The warning in the docs has been removed, the the Guidelines section remains intact. I did not find anything regarding outside click what needed to be updated there.
Why are we making this change?
To align with
currentmodern UX expectations.Impact to users
🟢 No impact.The prop defaults tofalse. 🟡 The type foronClose/onCancelprops changed. This will cause type check fails.onCloseforEuiModalPropsandonCancelforEuiConfirmModalProps:onCancel: ( event?: | React.KeyboardEvent<HTMLDivElement> | React.MouseEvent<HTMLButtonElement> + | MouseEvent + | TouchEvent ) => void;QA
Remove or strikethrough items that do not apply to your PR.
General checklist
Checked in both light and dark modesChecked in both MacOS and Windows high contrast modes(emulate forced colors if you do not have access to a Windows machine.)Checked in mobile@defaultif default values are missing) and playground togglesChecked Code Sandbox works for any docs examplesUpdated visual regression testsIf applicable, added the breaking change issue label (and filled out the breaking change checklist)If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)