feat: apply max-width in dialog #394

noahchoii · 2025-12-17T06:45:43Z

Description of Changes

applied dialog max-width
add dialog snapshots

Checklist

Before submitting the PR, please make sure you have checked all of the following items.

The PR title follows the Conventional Commits convention. (e.g., feat, fix, docs, style, refactor, test, chore)
I have added tests for my changes.
I have updated the Storybook or relevant documentation.
I have added a changeset for this change. (e.g., for any changes that affect users, such as component prop changes or new features).
I have performed a self-code review.
I have followed the project's coding conventions and component patterns.

changeset-bot · 2025-12-17T06:45:47Z

🦋 Changeset detected

Latest commit: 0da69fc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@vapor-ui/core	Patch
figma-plugin	Patch
website	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2025-12-17T06:45:48Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
vapor-ui	Ready	Preview, Comment	Dec 24, 2025 6:29am

gemini-code-assist · 2025-12-17T06:45:57Z

Summary of Changes

Hello @noahchoii, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the dialog component by introducing a max-width constraint, preventing it from occupying the full viewport width on larger displays. This change improves the user experience by maintaining a consistent and readable dialog size. Additionally, the Storybook examples have been expanded to thoroughly demonstrate the dialog's behavior across various predefined sizes, ensuring visual consistency and facilitating future development.

Highlights

Dialog Max-Width Implementation: The dialog component now applies a max-width to ensure it does not stretch excessively on larger screens, improving its responsiveness and visual presentation.
Transition Property Refinement: The transition property for dialogs has been updated to explicitly define transform and opacity for smoother animations.
Dialog Size Unit Conversion: The fixed width values for md, lg, and xl dialog sizes have been converted from rem units to px units for more precise control.
Storybook Updates: The Storybook for the dialog component has been updated with a new 'TestBed' story that showcases dialogs of different sizes (md, lg, xl) to verify the new max-width and size adjustments.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

안녕하세요. 다이얼로그 컴포넌트에 max-width를 적용하고 관련 스타일 및 스토리를 업데이트하는 PR을 리뷰했습니다. 전반적으로 좋은 변경이라고 생각합니다. 특히 transition 속성을 transitionProperty와 transitionDuration으로 명시적으로 분리하여 성능을 개선한 점이 인상적입니다. 몇 가지 개선점을 제안드립니다. CSS 파일에서 하드코딩된 px 값과 매직 넘버를 테마 변수로 대체하여 디자인 시스템의 일관성을 높이는 것을 고려해 주세요. 또한, Storybook 파일의 중복 코드를 리팩토링하여 가독성과 유지보수성을 향상시킬 수 있습니다. 자세한 내용은 각 파일의 주석을 참고해 주세요.

I am having trouble creating individual review comments. Click here to see my feedback.

packages/core/src/components/dialog/dialog.css.ts (57-59)

width 값을 rem에서 px 단위의 하드코딩된 값으로 변경하셨습니다. 스타일 가이드 7.3절에 따라 디자인 토큰은 CSS 변수(vars)를 통해 참조하고 하드코딩을 지양해야 합니다.

500px, 800px, 1140px와 같은 값들은 테마 변수로 관리하는 것이 좋습니다.
만약 적절한 테마 변수가 없다면, rem 단위를 유지하는 것을 고려해 보세요. rem 단위는 루트 폰트 크기에 따라 확장되므로 접근성 면에서 더 유연한 레이아웃을 제공합니다.

            md: layerStyle('components', { width: '31.25rem' }),
            lg: layerStyle('components', { width: '50rem' }),
            xl: layerStyle('components', { width: '71.25rem' }),

References

디자인 토큰(색상, 폰트, 간격 등)은 하드코딩하는 대신 CSS 변수(vars 객체)를 통해 참조해야 합니다. 이는 일관성을 유지하고 유지보수를 용이하게 합니다. ^(link)

packages/core/src/components/dialog/dialog.css.ts (36)

64px는 매직 넘버로 보입니다. 화면 양쪽의 여백을 의도한 것이라면, 테마 변수에 정의된 값을 사용하는 것이 좋습니다. 예를 들어, calc.multiply(vars.spacing.someValue, 2)와 같이 사용하면 유지보수성이 향상되고 디자인 시스템의 일관성을 지킬 수 있습니다.

References

디자인 토큰(색상, 폰트, 간격 등)은 하드코딩하는 대신 CSS 변수(vars 객체)를 통해 참조해야 합니다. 이는 일관성을 유지하고 유지보수를 용이하게 합니다. ^(link)

packages/core/src/components/dialog/dialog.stories.tsx (40-90)

TestBed 스토리에서 각 다이얼로그 크기(xl, lg, md)를 보여주기 위해 코드가 많이 중복되고 있습니다. 가독성과 유지보수성을 높이기 위해 이를 리팩토링하는 것이 좋겠습니다. 예를 들어, 사이즈 배열을 순회하며 컴포넌트를 렌더링하는 방식으로 개선할 수 있습니다.

<>
    {(['xl', 'lg', 'md'] as const).map((size) => (
        <Dialog.Root {...args} key={size} open size={size}>
            <Dialog.Trigger>hihi</Dialog.Trigger>
            <Dialog.PortalPrimitive>
                <Dialog.OverlayPrimitive />
                <Dialog.PopupPrimitive>
                    <Dialog.Header>
                        <Dialog.Title>다이얼로그입니다.</Dialog.Title>
                    </Dialog.Header>
                    <Dialog.Body>
                        <Dialog.Description>기본 형태의 다이얼로그입니다.</Dialog.Description>
                    </Dialog.Body>
                    <Dialog.Footer>
                        <Dialog.Close render={<Button colorPalette="contrast">닫기</Button>} />
                    </Dialog.Footer>
                </Dialog.PopupPrimitive>
            </Dialog.PortalPrimitive>
        </Dialog.Root>
    ))}
</>

vapor-ui · 2025-12-17T06:53:18Z

✅ All tests passed!

Tests	Passed	Failed	Duration	Report
144	144	0	2m 46s	Open report ↗︎

Click here if you need to update snapshots.

MaxLee-dev · 2025-12-18T14:18:17Z

packages/core/src/components/dialog/dialog.css.ts

    },
 });

+const SPACING = '32px';


Is there a reason it's set to 32px here? And how about getting the spacing from our vars using space-400?

I also debated whether to use tokens, but since this wasn't a design decision defined with the designer, but rather an arbitrary value added by the development team to prevent exceeding the browser width, I didn't use tokens!

MaxLee-dev · 2025-12-18T14:19:03Z

packages/core/src/components/dialog/dialog.css.ts

        flexDirection: 'column',
        alignItems: 'flex-start',

+        maxWidth: calc.subtract('100dvw', calc.multiply(SPACING, 2)),


Is there a reason dvw is specified here? For height below, using svh would be more consistent, though vw should always be the same anyway.

I believe dv*, sv*, and lv* should use appropriate values depending on the situation. Since svw is based on the smallest browser width and doesn't change dynamically, dvw is correct in this case because it does change dynamically. If you have a different opinion or think I've missed something, please let me know!

What I'm curious about is that height is set to svh, while width is set to dvw. Since vw doesn't actually change on mobile, it probably doesn't matter.

I determined that the viewport width (vw) could change when unfolding the screen on devices like the Galaxy Jet Flip or when rotating the device horizontally to enter landscape mode. I initially thought this wouldn't happen with svw, but when I actually tested it, it worked fine even with svw...? So, as you suggested, it would be best to standardize it. I'll make the correction!

MaxLee-dev · 2025-12-18T14:26:47Z

packages/core/src/components/dialog/dialog.stories.tsx

 };

-export const AA: Story = {
+export const TestBed: Story = {


[NIT]
This is just a suggestion, but I noticed that it's forced to open in the testBed, so it's always open in the Dialog docs. It might be good to add the code below to disable only this component.

export const TestBed: Story = { parameters: { docs: { disable: true }, },

Oh, I didn't realize that option was available. Thanks for the suggestion!

MaxLee-dev

👍

ZeroChoi2781 · 2025-12-24T06:26:24Z

packages/core/src/components/dialog/dialog.css.ts

    },
 });

+const SPACING = '32px';


It must be modified with rem.

noahchoii added 3 commits December 17, 2025 15:38

apply dialog max width

0506e9f

add dialog snapshots

a27e285

add changeset

a0151f2

noahchoii added the scope: dialog Issues related to the Dialog component. label Dec 17, 2025

noahchoii requested review from MaxLee-dev and ZeroChoi2781 as code owners December 17, 2025 06:45

px -> rem

4abfad7

gemini-code-assist bot reviewed Dec 17, 2025

View reviewed changes

vercel bot deployed to Preview December 17, 2025 06:49 View deployment

enhance story readability

e85ec78

remove magic number

68a9f80

vercel bot deployed to Preview December 17, 2025 06:59 View deployment

MaxLee-dev reviewed Dec 18, 2025

View reviewed changes

prevent to open the dialogs in docs

0615e93

vercel bot deployed to Preview December 19, 2025 00:26 View deployment

dvw -> svw / apply maxHeight for popup, not for body

9e6253f

noahchoii requested a review from MaxLee-dev December 19, 2025 06:27

vercel bot deployed to Preview December 19, 2025 06:28 View deployment

MaxLee-dev approved these changes Dec 23, 2025

View reviewed changes

ZeroChoi2781 reviewed Dec 24, 2025

View reviewed changes

packages/core/src/components/dialog/dialog.css.ts Outdated

},

});

const SPACING = '32px';

Copy link

Contributor

ZeroChoi2781 Dec 24, 2025

Choose a reason for hiding this comment

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

It must be modified with rem.

px -> rem

0da69fc

vercel bot deployed to Preview December 24, 2025 06:29 View deployment

noahchoii merged commit b6bd4db into main Dec 24, 2025
13 checks passed

noahchoii deleted the dialog-maxSize branch December 24, 2025 06:45

feat: apply max-width in dialog #394

feat: apply max-width in dialog #394

Uh oh!

Conversation

noahchoii commented Dec 17, 2025

Description of Changes

Checklist

Uh oh!

changeset-bot bot commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

vercel bot commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Dec 17, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

packages/core/src/components/dialog/dialog.css.ts (57-59)

packages/core/src/components/dialog/dialog.css.ts (36)

packages/core/src/components/dialog/dialog.stories.tsx (40-90)

Uh oh!

vapor-ui commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaxLee-dev Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaxLee-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

changeset-bot bot commented Dec 17, 2025 •

edited

Loading

vercel bot commented Dec 17, 2025 •

edited

Loading

vapor-ui commented Dec 17, 2025 •

edited

Loading

MaxLee-dev Dec 19, 2025 •

edited

Loading