refactor(qvism-preset): adopt inset shorthand and logical inline/block shorthands#1744
Open
te6-in wants to merge 3 commits into
Open
refactor(qvism-preset): adopt inset shorthand and logical inline/block shorthands#1744te6-in wants to merge 3 commits into
te6-in wants to merge 3 commits into
Conversation
…ions Collapse top/right/bottom/left into the inset shorthand only where all four sides exist in the same style object with the same value. Chrome 88 / Safari 15 baseline supports inset, so no fallback is needed. Regenerated css/recipes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (10)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughqvism-preset의 여러 레시피와 stackflow 스타일에서 절대 위치 지정과 좌우/상하 패딩·마진을 논리 속성과 inset 축약형으로 바꿨습니다. 공개 API 변경은 없습니다. Changes논리 속성 및 inset 전환
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Alpha Preview (Docs)
|
…tric pairs Merge symmetric physical longhand pairs into logical 2-value shorthands where both ends of an axis share the same value: paddingLeft/Right -> paddingInline, paddingTop/Bottom -> paddingBlock, and the margin/inset equivalents. Only pairs with identical values are merged; single-side and asymmetric declarations are left as physical properties. Icon-util props (IconProps) are excluded. Regenerated css/recipes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
te6-in
changed the title
Contributor
Alpha Preview (Stackflow SPA)
|
Contributor
Alpha Preview (Storybook)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/qvism-preset/src/recipes/list-item.ts (1)
126-167: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
insetInline전환 후transitionProperty의left, right가 더 이상 매칭되지 않음.
::before의transitionProperty는 여전히"background-color, left, right, border-radius"로 선언되어 있지만(Line 133), engaged/hover/active 상태 전환 시 실제로 값이 바뀌는 속성은insetInline(Line 142, 154, 163)입니다.inset-inline은left/right와 별개의 CSS 속성명이므로 transition-property 매칭에 실패하며, 눌림 배경의 좌우 확장/축소 애니메이션이 더 이상 부드럽게 전환되지 않고 즉시 바뀌는 회귀가 발생합니다.🐛 제안: transitionProperty를 논리 속성 이름으로 업데이트
transitionProperty: "background-color, left, right, border-radius", + transitionProperty: "background-color, inset-inline, border-radius",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/qvism-preset/src/recipes/list-item.ts` around lines 126 - 167, The `::before` transition setup in `list-item.ts` is still using physical properties (`left, right`) even though the engaged/hover/active states now animate `insetInline`; update the `transitionProperty` in the `::before` style so it matches the logical inset property used by the `[pseudo(":is(button, a)", not(disabled), engaged, "::before")]`, `media.isHoverableInputDevice`, and `media.isNotHoverableInputDevice` rules, ensuring the pressed-state expansion animation transitions smoothly again.packages/qvism-preset/src/recipes/side-navigation.ts (1)
196-208: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
paddingInline전환 시transition: padding이 더 이상 애니메이션되지 않음.
transition-property에 남아있는padding은 물리적 longhand(padding-top/right/bottom/left)만 감지하는 이름이며,padding-inline(inline-start/end)은 별도의 속성명이라 매칭되지 않습니다.root의 좌우 패딩이 이제paddingInline으로 선언되어 있고,collapsed상태에서 8px → 10px로 값이 바뀌는데(Line 254), 이 전환이 더 이상 부드럽게 애니메이션되지 않고 즉시 값이 바뀌는 시각적 회귀가 발생합니다.🐛 제안: transition-property를 논리 속성 이름으로 업데이트
transition: `padding ${duration}, background-color ${duration}`, + // paddingInline으로 전환되었으므로 논리 속성 이름을 명시 + transition: `padding-inline ${duration}, background-color ${duration}`,Also applies to: 253-255
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/qvism-preset/src/recipes/side-navigation.ts` around lines 196 - 208, The side navigation transition is still targeting physical padding longhands, so the `paddingInline` change in `side-navigation.ts` no longer animates smoothly. Update the `transition` in the style block that defines `root`/`collapsed` behavior to use the logical property name for horizontal padding instead of `padding`, keeping the existing background transition intact. Use the `paddingInline` declaration and the `collapsed` state padding change as the reference points when adjusting the transition property list.
🧹 Nitpick comments (1)
packages/qvism-preset/src/recipes/chip.ts (1)
52-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value참고: 동일 파일 내 물리적 padding이 남아있음.
label/root는paddingInline으로 전환됐지만prefixIcon/suffixIcon은 여전히paddingLeft/paddingRight(단측)를 사용합니다. 이번 PR 범위(동일 4방향 값의 inset 단축 표기)에는 해당하지 않지만, 추후 논리 속성 전환 작업 시paddingInlineStart/paddingInlineEnd로 함께 정리하면 좋겠습니다.Also applies to: 63-63
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/qvism-preset/src/recipes/chip.ts` at line 52, In chip.ts, the prefixIcon and suffixIcon styles still use physical one-sided padding via paddingLeft and paddingRight while label/root already use logical inset shorthands. Update the chip recipe’s prefixIcon/suffixIcon definitions to the logical equivalents paddingInlineStart and paddingInlineEnd so the sizing/padding approach is consistent across the same recipe and easier to maintain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/qvism-preset/src/recipes/list-item.ts`:
- Around line 126-167: The `::before` transition setup in `list-item.ts` is
still using physical properties (`left, right`) even though the
engaged/hover/active states now animate `insetInline`; update the
`transitionProperty` in the `::before` style so it matches the logical inset
property used by the `[pseudo(":is(button, a)", not(disabled), engaged,
"::before")]`, `media.isHoverableInputDevice`, and
`media.isNotHoverableInputDevice` rules, ensuring the pressed-state expansion
animation transitions smoothly again.
In `@packages/qvism-preset/src/recipes/side-navigation.ts`:
- Around line 196-208: The side navigation transition is still targeting
physical padding longhands, so the `paddingInline` change in
`side-navigation.ts` no longer animates smoothly. Update the `transition` in the
style block that defines `root`/`collapsed` behavior to use the logical property
name for horizontal padding instead of `padding`, keeping the existing
background transition intact. Use the `paddingInline` declaration and the
`collapsed` state padding change as the reference points when adjusting the
transition property list.
---
Nitpick comments:
In `@packages/qvism-preset/src/recipes/chip.ts`:
- Line 52: In chip.ts, the prefixIcon and suffixIcon styles still use physical
one-sided padding via paddingLeft and paddingRight while label/root already use
logical inset shorthands. Update the chip recipe’s prefixIcon/suffixIcon
definitions to the logical equivalents paddingInlineStart and paddingInlineEnd
so the sizing/padding approach is consistent across the same recipe and easier
to maintain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3b1d28fa-a1fe-418e-b656-d4acd4e47933
⛔ Files ignored due to path filters (90)
packages/css/all.cssis excluded by!packages/css/**/*packages/css/all.layered.cssis excluded by!packages/css/**/*packages/css/all.layered.min.cssis excluded by!packages/css/**/*packages/css/all.min.cssis excluded by!packages/css/**/*packages/css/recipes/accordion.cssis excluded by!packages/css/**/*packages/css/recipes/accordion.layered.cssis excluded by!packages/css/**/*packages/css/recipes/action-sheet-item.cssis excluded by!packages/css/**/*packages/css/recipes/action-sheet-item.layered.cssis excluded by!packages/css/**/*packages/css/recipes/action-sheet.cssis excluded by!packages/css/**/*packages/css/recipes/action-sheet.layered.cssis excluded by!packages/css/**/*packages/css/recipes/app-bar-main.cssis excluded by!packages/css/**/*packages/css/recipes/app-bar-main.layered.cssis excluded by!packages/css/**/*packages/css/recipes/app-bar.cssis excluded by!packages/css/**/*packages/css/recipes/app-bar.layered.cssis excluded by!packages/css/**/*packages/css/recipes/app-screen.cssis excluded by!packages/css/**/*packages/css/recipes/app-screen.layered.cssis excluded by!packages/css/**/*packages/css/recipes/attachment-input-item.cssis excluded by!packages/css/**/*packages/css/recipes/attachment-input-item.layered.cssis excluded by!packages/css/**/*packages/css/recipes/attachment-input.cssis excluded by!packages/css/**/*packages/css/recipes/attachment-input.layered.cssis excluded by!packages/css/**/*packages/css/recipes/badge.cssis excluded by!packages/css/**/*packages/css/recipes/badge.layered.cssis excluded by!packages/css/**/*packages/css/recipes/bottom-sheet.cssis excluded by!packages/css/**/*packages/css/recipes/bottom-sheet.layered.cssis excluded by!packages/css/**/*packages/css/recipes/callout.cssis excluded by!packages/css/**/*packages/css/recipes/callout.layered.cssis excluded by!packages/css/**/*packages/css/recipes/chip-tabs.cssis excluded by!packages/css/**/*packages/css/recipes/chip-tabs.layered.cssis excluded by!packages/css/**/*packages/css/recipes/chip.cssis excluded by!packages/css/**/*packages/css/recipes/chip.layered.cssis excluded by!packages/css/**/*packages/css/recipes/contextual-floating-button.cssis excluded by!packages/css/**/*packages/css/recipes/contextual-floating-button.layered.cssis excluded by!packages/css/**/*packages/css/recipes/dialog.cssis excluded by!packages/css/**/*packages/css/recipes/dialog.layered.cssis excluded by!packages/css/**/*packages/css/recipes/field.cssis excluded by!packages/css/**/*packages/css/recipes/field.layered.cssis excluded by!packages/css/**/*packages/css/recipes/floating-action-button.cssis excluded by!packages/css/**/*packages/css/recipes/floating-action-button.layered.cssis excluded by!packages/css/**/*packages/css/recipes/help-bubble.cssis excluded by!packages/css/**/*packages/css/recipes/help-bubble.layered.cssis excluded by!packages/css/**/*packages/css/recipes/image-frame-indicator.cssis excluded by!packages/css/**/*packages/css/recipes/image-frame-indicator.layered.cssis excluded by!packages/css/**/*packages/css/recipes/input-button.cssis excluded by!packages/css/**/*packages/css/recipes/input-button.layered.cssis excluded by!packages/css/**/*packages/css/recipes/layout.cssis excluded by!packages/css/**/*packages/css/recipes/layout.layered.cssis excluded by!packages/css/**/*packages/css/recipes/list-header.cssis excluded by!packages/css/**/*packages/css/recipes/list-header.layered.cssis excluded by!packages/css/**/*packages/css/recipes/list-item.cssis excluded by!packages/css/**/*packages/css/recipes/list-item.layered.cssis excluded by!packages/css/**/*packages/css/recipes/manner-temp-badge.cssis excluded by!packages/css/**/*packages/css/recipes/manner-temp-badge.layered.cssis excluded by!packages/css/**/*packages/css/recipes/menu-item.cssis excluded by!packages/css/**/*packages/css/recipes/menu-item.layered.cssis excluded by!packages/css/**/*packages/css/recipes/menu-sheet-item.cssis excluded by!packages/css/**/*packages/css/recipes/menu-sheet-item.layered.cssis excluded by!packages/css/**/*packages/css/recipes/menu-sheet.cssis excluded by!packages/css/**/*packages/css/recipes/menu-sheet.layered.cssis excluded by!packages/css/**/*packages/css/recipes/menu.cssis excluded by!packages/css/**/*packages/css/recipes/menu.layered.cssis excluded by!packages/css/**/*packages/css/recipes/notification-badge.cssis excluded by!packages/css/**/*packages/css/recipes/notification-badge.layered.cssis excluded by!packages/css/**/*packages/css/recipes/page-banner.cssis excluded by!packages/css/**/*packages/css/recipes/page-banner.layered.cssis excluded by!packages/css/**/*packages/css/recipes/reaction-button.cssis excluded by!packages/css/**/*packages/css/recipes/reaction-button.layered.cssis excluded by!packages/css/**/*packages/css/recipes/segmented-control.cssis excluded by!packages/css/**/*packages/css/recipes/segmented-control.layered.cssis excluded by!packages/css/**/*packages/css/recipes/select-box.cssis excluded by!packages/css/**/*packages/css/recipes/select-box.layered.cssis excluded by!packages/css/**/*packages/css/recipes/side-navigation-menu-item.cssis excluded by!packages/css/**/*packages/css/recipes/side-navigation-menu-item.layered.cssis excluded by!packages/css/**/*packages/css/recipes/side-navigation.cssis excluded by!packages/css/**/*packages/css/recipes/side-navigation.layered.cssis excluded by!packages/css/**/*packages/css/recipes/side-panel.cssis excluded by!packages/css/**/*packages/css/recipes/side-panel.layered.cssis excluded by!packages/css/**/*packages/css/recipes/slider-marker.cssis excluded by!packages/css/**/*packages/css/recipes/slider-marker.layered.cssis excluded by!packages/css/**/*packages/css/recipes/slider.cssis excluded by!packages/css/**/*packages/css/recipes/slider.layered.cssis excluded by!packages/css/**/*packages/css/recipes/snackbar-region.cssis excluded by!packages/css/**/*packages/css/recipes/snackbar-region.layered.cssis excluded by!packages/css/**/*packages/css/recipes/snackbar.cssis excluded by!packages/css/**/*packages/css/recipes/snackbar.layered.cssis excluded by!packages/css/**/*packages/css/recipes/tabs.cssis excluded by!packages/css/**/*packages/css/recipes/tabs.layered.cssis excluded by!packages/css/**/*packages/css/recipes/text-input.cssis excluded by!packages/css/**/*packages/css/recipes/text-input.layered.cssis excluded by!packages/css/**/*packages/css/recipes/toggle-button.cssis excluded by!packages/css/**/*packages/css/recipes/toggle-button.layered.cssis excluded by!packages/css/**/*
📒 Files selected for processing (37)
packages/qvism-preset/src/recipes/accordion.tspackages/qvism-preset/src/recipes/action-sheet-item.tspackages/qvism-preset/src/recipes/action-sheet.tspackages/qvism-preset/src/recipes/attachment-input.tspackages/qvism-preset/src/recipes/badge.tspackages/qvism-preset/src/recipes/bottom-sheet.tspackages/qvism-preset/src/recipes/callout.tspackages/qvism-preset/src/recipes/chip-tabs.tspackages/qvism-preset/src/recipes/chip.tspackages/qvism-preset/src/recipes/contextual-floating-button.tspackages/qvism-preset/src/recipes/dialog.tspackages/qvism-preset/src/recipes/field.tspackages/qvism-preset/src/recipes/floating-action-button.tspackages/qvism-preset/src/recipes/help-bubble.tspackages/qvism-preset/src/recipes/image-frame-indicator.tspackages/qvism-preset/src/recipes/input-button.tspackages/qvism-preset/src/recipes/layout.tspackages/qvism-preset/src/recipes/list-header.tspackages/qvism-preset/src/recipes/list-item.tspackages/qvism-preset/src/recipes/manner-temp-badge.tspackages/qvism-preset/src/recipes/menu-sheet-item.tspackages/qvism-preset/src/recipes/menu-sheet.tspackages/qvism-preset/src/recipes/menu.tspackages/qvism-preset/src/recipes/notification-badge.tspackages/qvism-preset/src/recipes/page-banner.tspackages/qvism-preset/src/recipes/reaction-button.tspackages/qvism-preset/src/recipes/segmented-control.tspackages/qvism-preset/src/recipes/select-box.tspackages/qvism-preset/src/recipes/side-navigation.tspackages/qvism-preset/src/recipes/side-panel.tspackages/qvism-preset/src/recipes/slider.tspackages/qvism-preset/src/recipes/snackbar.tspackages/qvism-preset/src/recipes/tabs.tspackages/qvism-preset/src/recipes/text-input.tspackages/qvism-preset/src/recipes/toggle-button.tspackages/qvism-preset/src/stackflow/app-bar.tspackages/qvism-preset/src/stackflow/app-screen.ts
✅ Files skipped from review due to trivial changes (13)
- packages/qvism-preset/src/recipes/layout.ts
- packages/qvism-preset/src/recipes/manner-temp-badge.ts
- packages/qvism-preset/src/recipes/floating-action-button.ts
- packages/qvism-preset/src/recipes/menu-sheet-item.ts
- packages/qvism-preset/src/recipes/reaction-button.ts
- packages/qvism-preset/src/recipes/segmented-control.ts
- packages/qvism-preset/src/recipes/list-header.ts
- packages/qvism-preset/src/recipes/toggle-button.ts
- packages/qvism-preset/src/recipes/chip-tabs.ts
- packages/qvism-preset/src/recipes/badge.ts
- packages/qvism-preset/src/recipes/notification-badge.ts
- packages/qvism-preset/src/recipes/text-input.ts
- packages/qvism-preset/src/stackflow/app-bar.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/qvism-preset/src/recipes/action-sheet.ts
- packages/qvism-preset/src/recipes/side-panel.ts
…tion-property The pressed/hover overlay ::before in list-item, menu, and accordion now sets its horizontal position via logical inset-inline, so name inset-inline in transition-property instead of the physical left/right. No behavior change (both animate identically; the browser resolves inset-inline to the physical computed value) - this only aligns the transitioned property with the declared one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
top/right/bottom/left에서inset중심으로 정리했습니다.paddingLeft/Right,paddingTop/Bottom에서paddingInline,paddingBlock으로 통일되어 RTL 등에서도 의도가 유지됩니다.