Fix modal focus #6278
Fix modal focus #6278
Conversation
| @@ -445,10 +444,9 @@ const Modal: BsPrefixRefForwardingComponent<'div', ModalProps> = | |||
|
|
|||
| const baseModalStyle = { ...style, ...modalStyle }; | |||
|
|
|||
| // Sets `display` always block when `animation` is false | |||
| if (!animation) { | |||
There was a problem hiding this comment.
Could not spot the difference in term of animation. I don't know why display was not always set to block.
You can slow down the animation with this (3s instead of 150ms):
.fade {
transition: opacity 3s linear;
}
There was a problem hiding this comment.
Looks like the logic was supposed to follow upstream bootstrap, although theirs appear to set display block after the animation finishes. I think the issue may be that we're running the autoFocus code in @restart/ui too early? Maybe we should be running it during one of the transition callbacks when animation is enabled.
cc @jquense
There was a problem hiding this comment.
Here the algorithm they are using:
// https://github.com/twbs/bootstrap/blob/v5.0.0/js/src/modal.js#L125-L129
scrollBarHide();
document.body.classList.add('modal-open');
///
// https://github.com/twbs/bootstrap/blob/v5.0.0/js/src/modal.js#L144
// https://github.com/twbs/bootstrap/blob/v5.0.0/js/src/util/backdrop.js#L46-L54
reflow(modalBackdrop);
modalBackdrop.classList.add('show');
await emulateAnimation(modalBackdrop);
///
// https://github.com/twbs/bootstrap/blob/v5.0.0/js/src/modal.js#L144
// https://github.com/twbs/bootstrap/blob/v5.0.0/js/src/modal.js#L243-L261
==> modal.style.display = 'block';
reflow(modal);
==> modal.classList.add('show');
enforceFocus();
///I think line modal.style.display = 'block' does not change anything because of
// https://github.com/twbs/bootstrap/blob/v5.1.3/scss/_transitions.scss#L4-L6
.fade:not(.show) {
opacity: 0;
}Line modal.classList.add('show') cancels opacity: 0.
I think they use display: none because
<div class="modal fade"><div class="modal-content">...</div></div> is already inside their DOM even if the modal is not displayed.
In React, <div class="modal fade">...</div> is added to the DOM only when the modal is displayed with prop show={true} via createPortal().
Check https://getbootstrap.com/docs/5.1/components/modal/ and remove display: none from .modal { ...; display: none; ...; }, you will see that you are unable to click anywhere on the page because of
.modal-dialog {
...;
pointer-events: none;
}There was a problem hiding this comment.
Reading https://developer.mozilla.org/en-US/docs/Web/CSS/display:
display: none
Turns off the display of an element so that it has no effect on layout (the document is rendered as though the element did not exist). All descendant elements also have their display turned off. To have an element take up the space that it would normally take, but without actually rendering anything, use the visibility property instead.
That's why display: none is not compatible with autoFocus on an input inside children.
So the issue may be that we're running the autoFocus code in @restart/ui too early? in my opinion won't fix this issue.
Check https://bugzilla.mozilla.org/show_bug.cgi?id=569955: elements with display: none; are not focusable. (they even have added a test for this :-) Add a test to make sure that autofocus on input elements does not work if they don't have a frame)
There was a problem hiding this comment.
Related Bootstrap issues: twbs/bootstrap#12525, twbs/bootstrap#22402
| <Form.Control | ||
| type="email" | ||
| placeholder="name@example.com" | ||
| autoFocus |
There was a problem hiding this comment.
autoFocus attribute on an input to demo the behavior
There was a problem hiding this comment.
seems ok. honestly i don't remember why we needed this style override originally.
|
btw, each time I modify Modal.tsx, the documentation refreshes (good) but it freezes my Chrome tab: I have to open a new tab each time 😱 |
|
Thanks for making this fix @tkrotoff |
Fix for #5102