fix(material/dialog): reserve room for touch targets in scrollable content - #33676
Open
ManicardiFrancesco wants to merge 1 commit into
Open
Conversation
…ntent `mat-dialog-content` scrolls, and its block-end padding is zero when the dialog has an actions row. Buttons, checkboxes and radios center a 48px touch target on a 40px control, and slide toggles center one on a 32px switch, so the target overhangs the control by 4-8px. An overhanging target at the end of the content is scrollable overflow, so a dialog whose content ends in one of those controls paints a scrollbar over content that fits. Reserve the overhang in the block-end padding instead. This keeps the touch targets at their accessible size, which the alternatives do not: disabling them (`touch-target-display: none`) shrinks the tap area, and anchoring them so they only overhang the block-start edge still left 4px of overflow in WebKit in local testing. Verified with a stock Material app in Chromium, Firefox and WebKit. A dialog with a trailing checkbox, radio, slide toggle or button goes from 4px, 4px, 8px and 4px of scrollable overflow respectively to 0 in all three engines. Dialogs with actions grow by 8px, which is the trade-off for not touching the tap areas. Fixes angular#29164.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this fixes
A dialog whose content ends with a button, checkbox, radio or slide toggle paints a vertical scrollbar over content that is fully visible.
mat-dialog-contentis a scroll container (overflow: auto) and its block-end padding is0when the dialog has an actions row (dialog-with-actions-content-padding: 20px 24px 0). Those controls center an absolutely positioned touch target on a smaller control — 48px on a 40px button/checkbox/radio, 48px on a 32px switch — so the target overhangs the control by 4–8px. An absolutely positioned box still counts towards its scroll container's block-end scrollable overflow, so the content scrolls by those invisible pixels.This is why the same dialog is fine without an actions row:
dialog-content-paddingends in20px, which absorbs the overhang.This change reserves the overhang in the block-end padding, so the touch targets keep their accessible size. It addresses the concern raised in #29164, that the control "is supposed to have a 40px height visually and layout-wise, but for accessibility reasons it needs to be a minimum of 48px" — the container gives the target room instead of the control giving up size.
Fixes #29164. Same root cause as #4764, #4748, #23565 and #26176.
Easy repro
Any dialog with an actions row and a trailing control:
The content fits, but the dialog scrolls by 4px.
document.querySelector('mat-dialog-content')reportsscrollHeight - clientHeight === 4, and the offending box is.mat-mdc-checkbox-touch-target. Removing the checkbox, or giving itmargin-bottom: 4px, makes the scrollbar go away — which is why this tends to get misdiagnosed as an app-level styling problem.The geometry alone reproduces without Angular, in any browser:
Verification
Stock Angular Material app, no custom CSS, default density, measured as
scrollHeight - clientHeightonmat-dialog-contentbefore and after the change:Trade-off, and how to avoid it
Dialogs with an actions row get 8px taller, and the gap between the content and the actions goes from 16px to 24px. If you would rather keep the current spacing exactly, the reservation can be compensated on the actions row, which leaves both the dialog height and the visible gap unchanged:
I left that out of the change because it overrides the block-start half of a consumer's own
dialog-actions-padding. Happy to add it, or to reserve only 4px (which covers buttons, checkboxes and radios but leaves 4px for slide toggles), if either is preferred.Alternatives that do not work
touch-target-display: noneon the controls inside dialog content — the workaround suggested in bug(material/button): Components with absolutely positioned touch targets (like mat-button) can cause scrollbar to appear. #26176. It fixes the scrollbar in every engine, but shrinks the tap area to the 40px control, which is the thing the touch target exists to prevent.elementFromPoint) and fixed Chromium and Firefox, but a stock dialog still measured 4px of overflow in WebKit, so it is not a reliable fix.overflow: clip+overflow-clip-marginon the control, to keep the target painted and hit-testable but out of the ancestor's overflow. Chromium and Firefox still reported the full 4px, and WebKit does not supportoverflow-clip-margin(CSS.supportsis false), where it clipped the target out of hit-testing instead —elementFromPointin the overhang returned the scroll container.:has()on the content to only pay the padding when a control is present — ruled out by the existing note indialog.scss, that the added specificity breaks internal clients.Notes
dialog-with-actions-content-paddingremains overridable.dialog.spec.tshas no layout/geometry assertions to extend, and this is not observable without real component CSS. Verified manually in the three engines as above.dialog-with-actions-content-paddingthere is20px 24px, which already absorbs the overhang.