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

[@mantine/core] fix: prevent onClose trigger on escape when modal is not open #6156

Merged
merged 1 commit into from
May 2, 2024

Conversation

hsskey
Copy link
Contributor

@hsskey hsskey commented May 1, 2024

Fix: Ensure onClose is not triggered by escape key when Modal is not open

This PR addresses a specific issue where the onClose function of the Modal component was incorrectly triggered by pressing the Escape key, even when the Modal was not actively open.

Problem:
When the Escape key was pressed, the onClose handler was executed regardless of the Modal's open state. This could unintentionally disrupt the user experience and application state if the onClose logic was not meant to be executed at that time.

Solution:
The fix involves a simple, yet effective modification in the useModal hook. By adding a conditional check that ensures the Modal is open (opened state is true) before invoking the onClose function, we prevent unwanted side effects

fixes #6148
commit: b6ea042

@rtivital rtivital merged commit 5ba2259 into mantinedev:master May 2, 2024
1 check passed
@rtivital
Copy link
Member

rtivital commented May 2, 2024

Thanks!

davydkov added a commit to likec4/likec4 that referenced this pull request May 18, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@mantine/core](https://mantine.dev/)
([source](https://togithub.com/mantinedev/mantine/tree/HEAD/packages/@mantine/core))
| [`^7.9.0` ->
`^7.9.1`](https://renovatebot.com/diffs/npm/@mantine%2fcore/7.9.0/7.9.1)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@mantine%2fcore/7.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@mantine%2fcore/7.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@mantine%2fcore/7.9.0/7.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@mantine%2fcore/7.9.0/7.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [@mantine/hooks](https://mantine.dev)
([source](https://togithub.com/mantinedev/mantine/tree/HEAD/packages/@mantine/hooks))
| [`^7.9.0` ->
`^7.9.1`](https://renovatebot.com/diffs/npm/@mantine%2fhooks/7.9.0/7.9.1)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@mantine%2fhooks/7.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@mantine%2fhooks/7.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@mantine%2fhooks/7.9.0/7.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@mantine%2fhooks/7.9.0/7.9.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>mantinedev/mantine (@&#8203;mantine/core)</summary>

###
[`v7.9.1`](https://togithub.com/mantinedev/mantine/releases/tag/7.9.1)

[Compare
Source](https://togithub.com/mantinedev/mantine/compare/7.9.0...7.9.1)

##### What's Changed

- `[@mantine/core]` Fix `theme.scale` being ignored in Input, Paper and
Table border styles
- `[@mantine/core]` Fix `virtualColor` function requring `use client` in
Next.js
- `[@mantine/core]` FloatingIndicator: Fix incorrect resize observer
logic
([#&#8203;6129](https://togithub.com/mantinedev/mantine/issues/6129))
- `[@mantine/core]` NumberInput: Fix incorrect `allowNegative` handling
with up/down arrows
([#&#8203;6170](https://togithub.com/mantinedev/mantine/issues/6170))
- `[@mantine/core]` Fix `error={true}` prop set on Checkbox, Radio and
Switch rendering unxpected error element with margin
- `[@mantine/core]` SegmentedControl: Fix `theme.primaryColor` not being
respected in the focus ring styles
- `[@mantine/core]` CloseButton: Fix incorrect specificity of some
selectors
- `[@mantine/core]` Fix incorrect `aria-label` handling in Select,
Autocomplete, MultiSelect and TagsInputs components
([#&#8203;6123](https://togithub.com/mantinedev/mantine/issues/6123))
- `[@mantine/core]` Modal: Prevent `onClose` from being called when
modal is not opened
([#&#8203;6156](https://togithub.com/mantinedev/mantine/issues/6156))
- `[@mantine/core]` PasswordInput: Fix duplicated password visibility
icon in Edge browser
([#&#8203;6126](https://togithub.com/mantinedev/mantine/issues/6126))
- `[@mantine/hooks]` use-hash: Fix hash value not being updated
correctly
([#&#8203;6145](https://togithub.com/mantinedev/mantine/issues/6145))
- `[@mantine/emotion]` Fix incorrect transform logic that was causing
extra hooks to render
([#&#8203;6159](https://togithub.com/mantinedev/mantine/issues/6159))

##### New Contributors

- [@&#8203;lachtanek](https://togithub.com/lachtanek) made their first
contribution in
[mantinedev/mantine#6145
- [@&#8203;hsskey](https://togithub.com/hsskey) made their first
contribution in
[mantinedev/mantine#6156
- [@&#8203;ataldev](https://togithub.com/ataldev) made their first
contribution in
[mantinedev/mantine#6198
- [@&#8203;OliverWales](https://togithub.com/OliverWales) made their
first contribution in
[mantinedev/mantine#6188
- [@&#8203;AustinWildgrube](https://togithub.com/AustinWildgrube) made
their first contribution in
[mantinedev/mantine#6170
- [@&#8203;theca11](https://togithub.com/theca11) made their first
contribution in
[mantinedev/mantine#6178

**Full Changelog**:
mantinedev/mantine@7.9.0...7.9.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/likec4/likec4).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjMuNSIsInVwZGF0ZWRJblZlciI6IjM3LjM2My41IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJkZXBlbmRlbmNpZXMiXX0=-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Denis Davydkov <denis@davydkov.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modal onClose runs even if Modal is not open
2 participants