Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This partially reverts commit 681bbe6. I talked through these changes at length with @smockle, but wanted to document some of the main points here.
The new
Dialog
component is intentionally allowing overflow (nooverflow: hidden
) because the main use case,ConfirmationDialog
, has buttons that go right to the edge of theDialog
. When those buttons are focused, the default browseroutline
falls outside theDialog
. Ifoverflow: hidden
is set, the outlines get chopped off. The only way to fix this specific issue is by allowing overflow. See #1203 for more details.In 681bbe6, we decided to make a similar adjustment to the
Overlay
component. The driving factor at the time wasoutline
clipping, similar to what we saw inDialog
, but specifically whenActionList.Item
s were focused within anOverlay
. Additionally, we wanted consistency betweenDialog
andOverlay
.After some new use cases have come up, such as scroll bars in
SelectPanel
, it's becoming clear that the overflow content will often cause the bottom corners to lose the correct border-radius, like this:Rather than expecting
Overlay
consumers to round their corners to perfectly match theOverlay
border radius, @smockle and I concluded it would be best to bring backoverflow: hidden
in theOverlay
. This gives us accurate border-radius, and ensures thatOverlay
contents aren't otherwise spilling out. We discussed some upcoming use cases, such asTooltip
s inOverlay
s, or nestedActionMenu
s as counter scenarios where overflow might be desirable. In all the cases we could think of, the real solution is to have those new overlaying elements be created in aPortal
, as theOverlay
is now. If portaled, those child elements will actually be outside theOverlay
in the DOM, and won't be restricted in any way by theoverflow: hidden
.