Popover: Add virtual padding to prevent it from hitting the viewport edge #69555

yogeshbhutkar · 2025-03-13T05:44:21Z

What?

This PR introduces a new overflowPadding prop for the Popover component, allowing precise control over the offset between the popover container and the viewport edge.

Why?

Previously, there was no way to adjust the offset when the popover container reached the viewport edge, often cutting it off or partially obscuring near screen boundaries.

How?

A new prop overflowPadding was introduced.
The value of this prop is passed down to the padding property within size middleware from floating-ui.

Testing Instructions

Navigate to Appearance -> Editor -> Templates.
Click on View Options.
Shrink the height of the Browser Window.
Confirm, the popover container doesn't overflow over the edit site page content.

Testing Instructions for Keyboard

Same.

Screenshots

Before

After

github-actions · 2025-03-13T06:51:46Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: stokesman <presstoke@git.wordpress.org>
Co-authored-by: okmttdhr <okat@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

mirka

Does this problem not affect the X axis? The current prop naming and descriptions feel a bit misleading on first glance because it only works on the Y axis.

I'm also wondering if these "padding" values (as Ariakit names it) should actually be set globally in some way, at least at the app level, if not at the wp-components level. I haven't assessed all the use cases, but it seems like this padded behavior is something all popovers would want by default?

yogeshbhutkar · 2025-03-14T05:54:35Z

Does this problem not affect the X axis?

I couldn’t find any instances where the popover overflows to the viewport edge along the X-axis. This is likely because, in most cases, the popover does not extend beyond the associated toggle button in that direction.

Precisely, this:

Ref. (from issue) "Although the dropdown is scrollable and remains functional, it is not visually clear that content is accessible via scrolling. Users may mistakenly believe that the menu is truncated instead of fully available."

Additionally, popovers rarely become scrollable along the X-axis, and when they do, it may not provide the best user experience. I'm interested in also knowing your thoughts on this.

The current prop naming and descriptions feel a bit misleading on first glance because it only works on the Y axis.

I've fixed this in 18f4007

It seems like this padded behavior is something all popovers would want by default?

Since most popovers don’t extend to the viewport edge due to their typically limited content, it might be best to handle these scenarios on a case-by-case basis. That said, applying a global padding style or setting the default overflowOffsetY to 8px should effectively address this. I’d love to hear your thoughts on this as well.

ciampo · 2025-03-14T09:59:37Z

Some quick thoughts:

I can't think of a case in Gutenberg (or in general) in which the popover would resize along the X axis — it usually flips, instead of resizing; At the same time, I don't think it would hurt to set the same padding for both axis?
Any chance we'd want the same "padding" functionality for flipping, too? That's what ariakit does

packages/components/src/popover/index.tsx

packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap

ciampo

The overall approach LGTM, I'll leave it up to the rest of the folks for final reviews and merging :)

t-hamano

This PR works well for me.

@yogeshbhutkar Finally, can you just resolve the conflict?

…lation

… component

…edge (WordPress#69555) * feat: introduce `overflowOffset` prop to control maximum height calculation * fix: set default value for `overflowOffset` prop in Popover component * chore: add changelog entry * fix: rename `overflowOffset` prop to `overflowOffsetY` for clarity * chore: update changelog to include `overflowOffsetY` prop for Popover component * fix: use `padding` instead of manual offset implementation * fix: add a static `8px` overflow padding * chore: move CHANGELOG * chore: update unit test snapshots * revert: restore changes made to snapshot file * fix: update maxHeight, maxWidth to accomodate overflowPadding * fix: remove maxWidth calculations * fix: use `padding` within `size` middleware * chore: move `CHANGELOG` to `Unreleased` section Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org> Co-authored-by: mirka <0mirka00@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: stokesman <presstoke@git.wordpress.org> Co-authored-by: okmttdhr <okat@git.wordpress.org>

yogeshbhutkar changed the title ~~Popover: Introduce overflowOffset prop to control maximum height calculation~~ Popover: Introduce overflowOffset prop to support adding an offset between popover and viewport edge Mar 13, 2025

yogeshbhutkar changed the title ~~Popover: Introduce overflowOffset prop to support adding an offset between popover and viewport edge~~ Popover: Introduce overflowOffset prop to support adding an offset Mar 13, 2025

yogeshbhutkar force-pushed the enhance-69553/overflow-offset-popover branch from b6f0322 to 15888d8 Compare March 13, 2025 06:23

yogeshbhutkar marked this pull request as ready for review March 13, 2025 06:51

yogeshbhutkar requested a review from ajitbohra as a code owner March 13, 2025 06:51

yogeshbhutkar mentioned this pull request Mar 13, 2025

Popover component: Improve Visibility When Hitting Viewport Edge #69553

Closed

t-hamano added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Mar 13, 2025

ciampo requested review from a team March 13, 2025 10:35

mirka reviewed Mar 13, 2025

View reviewed changes

yogeshbhutkar changed the title ~~Popover: Introduce overflowOffset prop to support adding an offset~~ Popover: Introduce overflowOffsetY prop to support adding an offset Mar 14, 2025

yogeshbhutkar force-pushed the enhance-69553/overflow-offset-popover branch from 06d0b01 to 18f4007 Compare March 14, 2025 04:58

ciampo reviewed Mar 14, 2025

View reviewed changes

packages/components/src/popover/index.tsx Outdated Show resolved Hide resolved

yogeshbhutkar changed the title ~~Popover: Introduce overflowOffsetY prop to support adding an offset~~ Popover: Introduce overflowPadding prop to support adding an offset Mar 18, 2025

yogeshbhutkar requested a review from ciampo March 18, 2025 06:34

yogeshbhutkar force-pushed the enhance-69553/overflow-offset-popover branch 2 times, most recently from fc04545 to 784fb4d Compare March 18, 2025 10:58

ciampo reviewed Mar 18, 2025

View reviewed changes

packages/components/src/popover/index.tsx Outdated Show resolved Hide resolved

yogeshbhutkar force-pushed the enhance-69553/overflow-offset-popover branch from 784fb4d to 0cd2051 Compare March 19, 2025 04:15

yogeshbhutkar force-pushed the enhance-69553/overflow-offset-popover branch from 0cd2051 to efe6ec6 Compare April 8, 2025 06:17

yogeshbhutkar requested a review from peterwilsoncc as a code owner April 8, 2025 06:33

yogeshbhutkar commented Apr 8, 2025

View reviewed changes

packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap Outdated Show resolved Hide resolved

ciampo reviewed Apr 8, 2025

View reviewed changes

yogeshbhutkar force-pushed the enhance-69553/overflow-offset-popover branch from d1c2b6f to 17f69c8 Compare April 15, 2025 04:35

t-hamano approved these changes Apr 17, 2025

View reviewed changes

feat: introduce overflowOffset prop to control maximum height calcu…

5ef5f9d

…lation

yogeshbhutkar added 13 commits April 17, 2025 13:54

fix: set default value for overflowOffset prop in Popover component

0a7e460

chore: add changelog entry

4c523e1

fix: rename overflowOffset prop to overflowOffsetY for clarity

5b04232

chore: update changelog to include overflowOffsetY prop for Popover…

a4c8efb

… component

fix: use padding instead of manual offset implementation

ba993e6

fix: add a static 8px overflow padding

9bc97a6

chore: move CHANGELOG

20f1778

chore: update unit test snapshots

4cf08ae

revert: restore changes made to snapshot file

5a7d962

fix: update maxHeight, maxWidth to accomodate overflowPadding

0c6699f

fix: remove maxWidth calculations

4523c3e

fix: use padding within size middleware

ad9cfce

chore: move CHANGELOG to Unreleased section

db5b287

yogeshbhutkar force-pushed the enhance-69553/overflow-offset-popover branch from 6637dfb to db5b287 Compare April 17, 2025 08:29

t-hamano changed the title ~~Popover: Introduce overflowPadding prop to support adding an offset~~ Popover: Add virtual padding to prevent it from hitting the viewport edge Apr 17, 2025

t-hamano merged commit feb44ab into WordPress:trunk Apr 17, 2025
59 checks passed

github-actions bot added this to the Gutenberg 20.8 milestone Apr 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Popover: Add virtual padding to prevent it from hitting the viewport edge #69555

Popover: Add virtual padding to prevent it from hitting the viewport edge #69555

Uh oh!

yogeshbhutkar commented Mar 13, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Mar 13, 2025 •

edited

Loading

Uh oh!

mirka left a comment

Uh oh!

yogeshbhutkar commented Mar 14, 2025 •

edited

Loading

Uh oh!

ciampo commented Mar 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ciampo left a comment

Uh oh!

t-hamano left a comment

Uh oh!

Uh oh!

Uh oh!

Popover: Add virtual padding to prevent it from hitting the viewport edge #69555

Popover: Add virtual padding to prevent it from hitting the viewport edge #69555

Uh oh!

Conversation

yogeshbhutkar commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots

Before

After

Uh oh!

github-actions bot commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mirka left a comment

Choose a reason for hiding this comment

Uh oh!

yogeshbhutkar commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ciampo commented Mar 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ciampo left a comment

Choose a reason for hiding this comment

Uh oh!

t-hamano left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

yogeshbhutkar commented Mar 13, 2025 •

edited

Loading

github-actions bot commented Mar 13, 2025 •

edited

Loading

yogeshbhutkar commented Mar 14, 2025 •

edited

Loading