Skip to content

feat(ui-modal): add margin to fullscreen modal#2608

Merged
git-nandor merged 2 commits into
masterfrom
INSTUI-5067_add_margin_to_modal
Jul 16, 2026
Merged

feat(ui-modal): add margin to fullscreen modal#2608
git-nandor merged 2 commits into
masterfrom
INSTUI-5067_add_margin_to_modal

Conversation

@git-nandor

@git-nandor git-nandor commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

INSTUI-5067

Summary

A size="fullscreen" Modal previously covered the entire viewport edge-to-edge, making it hard to tell it was a modal.

Changes

  • Modal applies fullScreenMargin (new token) based on sharedTokens.breakpoints (new token).
  • The Small viewports docs example now uses a theme override to simulate the small viewport media query behavior.
  • The Constraining Modal to a parent element section example was updated to fix the scrollbar introduced by the new margin changes by increasing its size.
  • prettier@3.8.1 pulled by new style-dictionary@4.4.0 caused conflicts with ui-codemods. Fixed by adding "prettier": "2.8.8" to the root package.json.

Test plan

Check the Modal page Media Modal example with default (fullscreen, cover) settings.

  • Large screen (≥ 768px): it is inset on all sides, rounded corners + shadow, backdrop visible around it.
  • Small screen (< 768px): edge-to-edge, no margin/radius.

Co-Authored-By: 🤖 Claude

@git-nandor git-nandor changed the title feat(ui-modal): on larger screens, inset the fullscreen modal from th… feat(ui-modal): add margin to fullscreen modal Jun 24, 2026
@git-nandor
git-nandor force-pushed the INSTUI-5067_add_margin_to_modal branch from 45e97b7 to e015aee Compare June 24, 2026 15:13
@git-nandor git-nandor self-assigned this Jun 24, 2026
@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-16 13:56 UTC

@github-actions

Copy link
Copy Markdown

Visual regression report

No changes.

Status Count
Unchanged 32
Changed 0
New 0
Removed 0

📊 View full report

Baselines come from the visual-baselines branch. They refresh on every merge to master.

github-actions Bot pushed a commit that referenced this pull request Jun 24, 2026
@git-nandor
git-nandor force-pushed the INSTUI-5067_add_margin_to_modal branch from e015aee to 5893625 Compare June 25, 2026 10:50
@git-nandor
git-nandor requested a review from joyenjoyer June 29, 2026 07:56
@git-nandor
git-nandor marked this pull request as ready for review June 29, 2026 07:57
@git-nandor
git-nandor requested review from balzss and removed request for balzss June 29, 2026 11:23

@joyenjoyer joyenjoyer left a comment

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.

Looks good to me visually. However, modal rendering became noticeably slower. Please check it (hint: use any browser's performance profiler).

It would be better to put the Prettier fix in a separate commit.

I ran Claude's code review skill against this PR, and it claimed that boxShadowObjectsToCSSString is being called redundantly multiple times. Please look into this as well.

@git-nandor
git-nandor force-pushed the INSTUI-5067_add_margin_to_modal branch 2 times, most recently from 670c845 to 12bcb78 Compare July 13, 2026 15:37
@git-nandor

Copy link
Copy Markdown
Contributor Author

Thanks! Prettier fix in a separate commit.
Redundant boxShadowObjectsToCSSString calls fixed by computing it once and reusing it.
Box-shadow blur is one of the most expensive paint operations and scales with painted area. Modal now paints a 2-layer, 28px-blur box-shadow + rounded corners on a near-fullscreen element so the cost shows up.

@git-nandor
git-nandor requested a review from joyenjoyer July 13, 2026 15:42
github-actions Bot pushed a commit that referenced this pull request Jul 13, 2026
git-nandor and others added 2 commits July 14, 2026 10:28
style-dictionary@4.4.0 pulls prettier@3.8.1, which conflicts with ui-codemods.
Pin prettier to 2.8.8 in the root package.json to resolve the version mismatch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e viewport edge for a11y

A size="fullscreen" Modal previously covered the entire viewport edge-to-edge,
making it hard to tell it was a modal. On viewports >= md it is now inset via the
new fullScreenMargin token, with rounded corners and box-shadow restored.

Also compute the box-shadow CSS string once per generateStyle call and reuse it at
both usage sites instead of recomputing it on every render.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@git-nandor
git-nandor force-pushed the INSTUI-5067_add_margin_to_modal branch from 12bcb78 to a967102 Compare July 14, 2026 08:28

@joyenjoyer joyenjoyer left a comment

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.

looks good now

@git-nandor
git-nandor requested a review from ToMESSKa July 14, 2026 14:03
@git-nandor
git-nandor merged commit e36b2b5 into master Jul 16, 2026
12 checks passed
@git-nandor
git-nandor deleted the INSTUI-5067_add_margin_to_modal branch July 16, 2026 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants