Skip to content

fix(dialog): honour contentPadding on fullscreen dialogs above 600px - #8112

Draft
tamas-sage wants to merge 1 commit into
masterfrom
fix/FE-7748-dialog-fullscreen-content-padding
Draft

fix(dialog): honour contentPadding on fullscreen dialogs above 600px#8112
tamas-sage wants to merge 1 commit into
masterfrom
fix/FE-7748-dialog-fullscreen-content-padding

Conversation

@tamas-sage

Copy link
Copy Markdown
Contributor

Proposed behaviour

The default padding is now skipped when a contentPadding prop is present, so the consumer's value (e.g. { p: 0 }) is honoured at all widths. Dialogs without contentPadding keep the existing default.

Current behaviour

On size="fullscreen" dialogs, contentPadding is ignored above 600px. Fullscreen dialogs always apply a responsive default side padding regardless of whether contentPadding was set, so contentPadding={{ p: 0 }} only works below 600px.

Checklist

  • Commits follow our style guide
  • Related issues linked in commit messages if required
  • Screenshots are included in the PR if useful
  • All themes are supported if required
  • Unit tests added or updated if required
  • Playwright automation tests added or updated if required
  • Storybook added or updated if required
  • Translations added or updated (including creating or amending translation keys table in storybook) if required
  • Typescript d.ts file added or updated if required
  • Related docs have been updated if required

QA

  • Tested in provided StackBlitz sandbox/Storybook
  • Add new Playwright test coverage if required
  • Carbon implementation matches Design System/designs
  • UI Tests GitHub check reviewed if required

Additional context

Testing instructions

@tamas-sage tamas-sage self-assigned this Aug 3, 2026
@tamas-sage
tamas-sage requested a review from a team as a code owner August 3, 2026 14:24
@tamas-sage
tamas-sage marked this pull request as draft August 3, 2026 14:24
Comment on lines +84 to +99
const contentPaddingKeys = [
"p",
"padding",
"px",
"paddingX",
"py",
"paddingY",
"pt",
"paddingTop",
"pr",
"paddingRight",
"pb",
"paddingBottom",
"pl",
"paddingLeft",
];

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.

suggestion(non-blocking): We have a config file in the __next__ Dialog. This could possibly live in there but I'm not precious about this. Happy for you to leave it for second reviewer to decide if this is a good idea or not.

@DipperTheDan

Copy link
Copy Markdown
Contributor

Just a reminder before you come to merge this, remember to include the original Github issue (#8103) in the commit message please. It's just so that the issue is automatically closed when this is merged 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants