Make Dialog header and footer sticky - #77182
Conversation
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @ManishDhorepatil. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @manishdhorepatil-art! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
ciampo
left a comment
There was a problem hiding this comment.
Hey @ManishDhorepatil , thank you for opening this PR.
The approach per se is not is not wrong, but unfortunately, the header and footer are looking off when content is scrolled
Did you test these changes yourself before asking for a review?
Here is a suggested approach
.popup {
--wp-ui-dialog-padding: var(--wpds-dimension-padding-2xl);
/* replace the existing hardcoded token reference */
padding: var(--wp-ui-dialog-padding);
/* ... rest of existing styles */
}
.header {
position: sticky;
top: 0;
z-index: 1;
background-color: var(--wpds-color-bg-surface-neutral-strong);
margin-inline: calc(-1 * var(--wp-ui-dialog-padding));
margin-top: calc(-1 * var(--wp-ui-dialog-padding));
padding-inline: var(--wp-ui-dialog-padding);
padding-top: var(--wp-ui-dialog-padding);
/* existing styles */
}
.footer {
position: sticky;
bottom: 0;
z-index: 1;
background-color: var(--wpds-color-bg-surface-neutral-strong);
margin-inline: calc(-1 * var(--wp-ui-dialog-padding));
margin-bottom: calc(-1 * var(--wp-ui-dialog-padding));
padding-inline: var(--wp-ui-dialog-padding);
padding-bottom: var(--wp-ui-dialog-padding);
/* existing styles */
}See the Card component (--wp-ui-card-padding) for an existing example of this pattern, including the negative-margin "fullbleed" technique.
Other points to address:
- Let's add a dedicated Storybook example for easier discovery and testing
- Let's add a
stickyprop toDialog.Footerto enable/disable sticky behaviour (@jameskoster , what do you think should the default behaviour be?) - @jameskoster , do you think we should display a border / shadow when content is scrolled, similarly to the
Modalcomponent in@wordpress/components?
Let's also solve rebase, solve conflicts, and add a CHANGELOG entry.
|
Generally the footer will include actions, so making it sticky seems like a reasonable default. We should show a border for now, but ideally I suppose this would be handled systematically by a dedicated |
|
Hey @manishdhorepatil-art , let me know if the feedback shared above makes sense, and if you still have capacity to work on this. |
|
Hey @manishdhorepatil-art , are you working on this? If not, please let me know—I’d like to try solving this issue. |
|
@vishnupprajapat i am working on this |
|
@ciampo chan you Please review this PR now? |
|
Hey @manishdhorepatil-art , I appreciate the effort, but this PR has a lot of aspects that need addressing: the layout is still quite broken, the react-context based approach is overkill (data-attributes are enough), spacing is off compared to trunk, and there are some build errors preventing the PR from building. Given that my time is limited, rather than leaving a very elaborate review, I've opened #77559 to implement an complete solution, and will close this PR. I will still make sure to list you as a co-author in the new PR. For future contributions, consider working on issues marked as "Good first issue". |
|
@ciampo >>I will still make sure to list you as a co-author in the new PR. |



What?
When dialog content overflows and the popup scrolls,
Dialog.Headerstays pinned to the top of the visible area andDialog.Footerstays pinned to the bottom, using sticky positioning and the dialog surface background so scrolling content does not show through.Why?
Long dialogs currently scroll the entire surface (including header and footer), so users lose context for the title and primary actions while reading. This matches the intent in #77180: parity with Modal-style behavior and clearer UX for scrollable dialogs.
How?
packages/ui/src/dialog/style.module.css,Dialog.Headerusesposition: sticky,top: 0,z-index: 1, andbackground-colorset to the dialog surface token.Dialog.Footerusesposition: sticky,bottom: 0, the samez-indexand background, preserving existing flex layout and spacing.Testing Instructions
npm installif needed, thennpm run storybook:dev.Dialog.HeaderandDialog.Footerso the popup scrolls vertically.Testing Instructions for Keyboard