-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(modal): add new prop to disable close button from modal (#17410) #17444
feat(modal): add new prop to disable close button from modal (#17410) #17444
Conversation
All contributors have signed the DCO. |
I have read the DCO document and I hereby sign the DCO. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17444 +/- ##
=======================================
Coverage 78.33% 78.33%
=======================================
Files 408 408
Lines 14015 14015
Branches 4324 4324
=======================================
Hits 10979 10979
Misses 2868 2868
Partials 168 168 ☔ View full report in Codecov by Sentry. |
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.
Hey thanks for suggesting this! Curious, why does onRequestClose
not work for your use case?
I'm not sure if disabling the close button is something we want to encourage from a ux or a11y perspective. I think the intent would be to use onRequestClose to stop the modal from closing when necessary and additionally adding an inline notification to inform the user why the modal can not be closed.
@laurenmrice what do you think?
If we land on this being a good idea we'll additionally need to override preventCloseOnClickOutside
to be true
.
@tay1orjones. It is not actually a case of mine, it is better explained in this issue #17410 |
@CaiqueSobral Ah, my mistake, I thought you were the original issue author. For now this isn't something we're going to add to Modal without strong reasoning due to the accessibility implications. Thanks again for your work here, I'm sorry to close this on you. Issues labelled
proposal: open
|
Closes #17410
Added a new boolean prop to Modal to not display the close button.
Changelog
New
Testing / Reviewing
The close button from a modal should not be displayed if "closeButtonDisabled" prop is set in the modal root