Skip to content

Premium Analytics: keep dialog focus rings from being clipped #52110

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.

Sign up for GitHub

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

Merged
chihsuan merged 2 commits into from
Sep 9, 2026
Merged

Premium Analytics: keep dialog focus rings from being clipped #52110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2fb98e
fix: keep Premium Analytics dialog focus rings from being clipped
chihsuan Sep 9, 2026
02976a4
fix: mirror the upstream chrome padding so dialog gaps match trunk
chihsuan Sep 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions projects/packages/premium-analytics/changelog/fix-pa-dialog-focus-ring-clipping
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Dialogs: keep the comment field's focus ring from being clipped by the dialog footer.

@dognose24 dognose24 Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a user-visible fix, so it should also get entries in the plugins that render the dashboard, matching what #52060, #51882 and #51772 did (plugins/jetpack and plugins/premium-analytics, plus plugins/wpcomsh in #51772). Otherwise it only reaches the package changelog. For example:

jp changelog add plugins/jetpack -s patch -t bugfix -e "Premium Analytics: keep the dialog focus ring from being clipped by the dialog footer."
jp changelog add plugins/premium-analytics -s patch -t fixed -e "Dialogs: keep the focus ring from being clipped by the dialog footer."

@chihsuan chihsuan Sep 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, the last few PA UI PRs all added those two. Added entries for plugins/jetpack and plugins/premium-analytics. Left wpcomsh out to match 52044 and 51873, happy to add it if you think Atomic users should see it.

6 changes: 0 additions & 6 deletions ...ackages/premium-analytics/routes/dashboard/components/feedback/feedback-modal.module.scss
View file
Open in desktop

This file was deleted.

3 changes: 1 addition & 2 deletions projects/packages/premium-analytics/routes/dashboard/components/feedback/feedback-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { __ } from '@wordpress/i18n';
*/
import { useTrackEvent } from '../../hooks/use-track-event';
import { FeedbackFields } from './feedback-fields';
import styles from './feedback-modal.module.scss';

// Reaches Happiness as the subject line of the feedback email ("Feedback received
// from …"), so it has to name the surface without any further context.
Expand Down Expand Up @@ -101,7 +100,7 @@ export function FeedbackModal( { onClose }: FeedbackModalProps ) {
) : (
<>
<Dialog.Content>
<Stack className={ styles.body } direction="column" gap="xl">
<Stack direction="column" gap="xl">
<FeedbackFields
rating={ rating }
onRatingChange={ setRating }
Expand Down
20 changes: 20 additions & 0 deletions projects/packages/premium-analytics/routes/dashboard/overlay-focus-ring.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Pinned chrome zeroes the scroll container's block padding on that edge,
// clipping a focus ring, which sits twice the focus width outside its control.
// Drop when @wordpress/ui ships https://github.com/WordPress/gutenberg/pull/82443.
header ~ [data-wp-ui-overlay-scroll-container] {
padding-block-start: calc(var(--wpds-border-width-focus) * 2);
}

[data-wp-ui-overlay-scroll-container]:has(~ footer) {
padding-block-end: calc(var(--wpds-border-width-focus) * 2);
}

@dognose24 dognose24 Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upstream (WordPress/gutenberg#82443) also subtracts the same clearance from the adjacent chrome padding, so the header-to-content and content-to-footer gaps stay unchanged. This shim only does the first half, so each pinned edge is currently 3px taller than trunk, and it will shrink back by that much when the @wordpress/ui bump drops the shim.

Mirroring the other half makes the bump a pure no-op:

header:has(~ [data-wp-ui-overlay-scroll-container]) {
	padding-block-end: calc(var(--wpds-dimension-gap-lg) - 1px - var(--wpds-border-width-focus) * 2);
}

[data-wp-ui-overlay-scroll-container] ~ footer {
	padding-block-start: calc(var(--wpds-dimension-gap-lg) - 1px - var(--wpds-border-width-focus) * 2);
}

The - 1px is the separator border slot 0.21.0 already reserves on pinned chrome; the whole value has to be restated because the shim cannot reference the layered rule's value.

@chihsuan chihsuan Sep 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, applied. I hadn't spotted that upstream trims the chrome side too, so the gaps were 3px wider than trunk. Mirroring both halves means the @wordpress/ui bump is a pure no-op, which is nicer.


// The chrome gives that clearance back, so the gap matches trunk. `- 1px` is
// the separator slot; an unlayered rule cannot read the layered value.
header:has(~ [data-wp-ui-overlay-scroll-container]) {
padding-block-end: calc(var(--wpds-dimension-gap-lg) - 1px - var(--wpds-border-width-focus) * 2);
}

[data-wp-ui-overlay-scroll-container] ~ footer {
padding-block-start: calc(var(--wpds-dimension-gap-lg) - 1px - var(--wpds-border-width-focus) * 2);
}
1 change: 1 addition & 0 deletions projects/packages/premium-analytics/routes/dashboard/stage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
useOnboarding,
useSectionDateFilter,
} from './hooks';
import './overlay-focus-ring.scss';
import styles from './stage.module.scss';
import type { DateRange, YearSurfacePresetId } from '@jetpack-premium-analytics/datetime';

Expand Down
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/fix-pa-dialog-focus-ring-clipping
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: bugfix

Premium Analytics: keep the dialog focus ring from being clipped by the dialog footer.
4 changes: 4 additions & 0 deletions projects/plugins/premium-analytics/changelog/fix-pa-dialog-focus-ring-clipping
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Dialogs: keep the focus ring from being clipped by the dialog footer.
Loading