Skip to content

Commit

Permalink
fix(styles): fix to explicitly initialize CSS variables (#1846)
Browse files Browse the repository at this point in the history
<!--
  How to write a good PR title:
- Follow [the Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/).
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

## Self Checklist

- [x] I wrote a PR title in **English** and added an appropriate
**label** to the PR.
- [x] I wrote the commit message in **English** and to follow [**the
Conventional Commits
specification**](https://www.conventionalcommits.org/en/v1.0.0/).
- [x] I [added the
**changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)
about the changes that needed to be released. (or didn't have to)
- [x] I wrote or updated **documentation** related to the changes. (or
didn't have to)
- [x] I wrote or updated **tests** related to the changes. (or didn't
have to)
- [x] I tested the changes in various browsers. (or didn't have to)
  - Windows: Chrome, Edge, (Optional) Firefox
  - macOS: Chrome, Edge, Safari, (Optional) Firefox

## Related Issue
<!-- Please link to issue if one exists -->

<!-- Fixes #0000 -->

## Summary
<!-- Please brief explanation of the changes made -->

#1837 에서 이어집니다. d074178 부터

CSS variable을 사용할 때 명시적으로 Rule 안에서 초깃값을 지정하도록 합니다.
 
## Details
<!-- Please elaborate description of the changes -->

- CSS variable(CSS Custom property)의 값은 상속의 대상이 되기 때문에, 명시적으로 값을 초기화해두지
않으면 부모의 값을 상속하게 됩니다.
- 이전 Box 등의 레이아웃 컴포넌트를 구현할 때, 공용 margin 속성 및 layout 속성에 초기화 대신 fallback
값으로만 기본값을 정의해두었습니다.
- 이렇게 구현할 경우, 부모에 같은 스타일이 있으면(e.g. `--b-margin: 10px`) 자식 엘리먼트가 fallback
값을 적용하지 않고 **부모의 스타일을 적용하게됩니다.**
- 이를 명시적으로 기본값을 부여하여 해결했습니다. 기본적으로 `initial` 키워드로 브라우저 기본값을 따랐고,
margin/padding 등 shorthand property에서는 shorthand property로 initial 키워드를
사용할 수 없으므로(`margin: initial initial initial initial => invalid`) MDN 문서를
참고하여 속성의 기본값을 직접 지정해주었습니다.
- border-style의 경우 기본값이 `none` 이므로, border-width가 있을 때 별도의 style 지정 없이도
바로 보일 수 있게 `solid` 로 지정했습니다.
- border-width의 경우 기본값이 `middle` 이므로, border-width가 없을 때 보이지 않도록 `0` 으로
지정했습니다.
 
### Breaking change? (Yes/No)
<!-- If Yes, please describe the impact and migration path for users -->

No

## References
<!-- Please list any other resources or points the reviewer should be
aware of -->

-
https://developer.mozilla.org/ko/docs/Web/CSS/Using_CSS_custom_properties#%EC%82%AC%EC%9A%A9%EC%9E%90_%EC%A7%80%EC%A0%95_%EC%86%8D%EC%84%B1%EC%9D%98_%EC%83%81%EC%86%8D
-
https://github.com/Shopify/polaris/blob/main/polaris-react/src/components/Box/Box.module.scss
  • Loading branch information
sungik-choi authored Dec 20, 2023
1 parent 16ad5b9 commit ca47057
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 44 deletions.
5 changes: 5 additions & 0 deletions .changeset/quick-peaches-fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@channel.io/bezier-react": minor
---

Fixes style inheritance issues by explicitly giving CSS custom properties an initial value.
4 changes: 3 additions & 1 deletion packages/bezier-react/src/components/Stack/Stack.module.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
@layer components {
.Stack {
--b-stack-spacing: initial;

box-sizing: border-box;
gap: var(--b-stack-spacing, normal);
gap: var(--b-stack-spacing);

&.display-flex {
display: flex;
Expand Down
4 changes: 3 additions & 1 deletion packages/bezier-react/src/components/Text/Text.module.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
@layer components {
.Text {
color: var(--b-text-color, inherit);
--b-text-color: inherit;

color: var(--b-text-color);
font-style: normal;
font-weight: var(--font-weight-400);
transition: color var(--transition-s);
Expand Down
102 changes: 62 additions & 40 deletions packages/bezier-react/src/components/shared.module.scss
Original file line number Diff line number Diff line change
@@ -1,49 +1,71 @@
@layer components {
.margin {
--margin-all: var(--b-margin-all, 0);
--b-margin: 0;
--b-margin-x: var(--b-margin);
--b-margin-y: var(--b-margin);
--b-margin-top: var(--b-margin-y);
--b-margin-right: var(--b-margin-x);
--b-margin-bottom: var(--b-margin-y);
--b-margin-left: var(--b-margin-x);

margin:
var(--b-margin-top, var(--b-margin-y, var(--margin-all)))
var(--b-margin-right, var(--b-margin-x, var(--margin-all)))
var(--b-margin-bottom, var(--b-margin-y, var(--margin-all)))
var(--b-margin-left, var(--b-margin-x, var(--margin-all)));
margin: var(--b-margin-top) var(--b-margin-right) var(--b-margin-bottom) var(--b-margin-left);
}

.layout {
--padding-all: var(--b-padding-all, 0);
--border-width: var(--b-border-width, 0);
--overflow: var(--b-overflow, visible);
--b-padding: 0;
--b-padding-x: var(--b-padding);
--b-padding-y: var(--b-padding);
--b-padding-top: var(--b-padding-y);
--b-padding-right: var(--b-padding-x);
--b-padding-bottom: var(--b-padding-y);
--b-padding-left: var(--b-padding-x);
--b-width: initial;
--b-height: initial;
--b-max-width: initial;
--b-max-height: initial;
--b-min-width: initial;
--b-min-height: initial;
--b-position: initial;
--b-inset: auto;
--b-top: var(--b-inset);
--b-right: var(--b-inset);
--b-bottom: var(--b-inset);
--b-left: var(--b-inset);
--b-shrink: initial;
--b-grow: initial;
--b-bg-color: initial;
--b-border-color: initial;
--b-border-radius: initial;
--b-border-width: 0;
--b-border-top-width: var(--b-border-width);
--b-border-right-width: var(--b-border-width);
--b-border-bottom-width: var(--b-border-width);
--b-border-left-width: var(--b-border-width);
--b-border-style: solid;
--b-elevation: initial;
--b-z-index: initial;
--b-overflow: initial;
--b-overflow-x: var(--b-overflow);
--b-overflow-y: var(--b-overflow);

padding:
var(--b-padding-top, var(--b-padding-y, var(--padding-all)))
var(--b-padding-right, var(--b-padding-x, var(--padding-all)))
var(--b-padding-bottom, var(--b-padding-y, var(--padding-all)))
var(--b-padding-left, var(--b-padding-x, var(--padding-all)));
width: var(--b-width, auto);
height: var(--b-height, auto);
max-width: var(--b-max-width, none);
max-height: var(--b-max-height, none);
min-width: var(--b-min-width, none);
min-height: var(--b-min-height, none);
position: var(--b-position, static);
inset: var(--b-inset, auto);
top: var(--b-top, auto);
right: var(--b-right, auto);
bottom: var(--b-bottom, auto);
left: var(--b-left, auto);
flex-shrink: var(--b-shrink, 1);
flex-grow: var(--b-grow, 0);
background-color: var(--b-bg-color, transparent);
border-color: var(--b-border-color, transparent);
border-radius: var(--b-border-radius, 0);
border-top-width: var(--b-border-top-width, var(--border-width));
border-right-width: var(--b-border-right-width, var(--border-width));
border-bottom-width: var(--b-border-bottom-width, var(--border-width));
border-left-width: var(--b-border-left-width, var(--border-width));
border-style: var(--b-border-style, solid);
box-shadow: var(--b-elevation, none);
z-index: var(--b-z-index, auto);
overflow-x: var(--b-overflow-x, var(--overflow));
overflow-y: var(--b-overflow-y, var(--overflow));
padding: var(--b-padding-top) var(--b-padding-right) var(--b-padding-bottom) var(--b-padding-left);
width: var(--b-width);
height: var(--b-height);
max-width: var(--b-max-width);
max-height: var(--b-max-height);
min-width: var(--b-min-width);
min-height: var(--b-min-height);
position: var(--b-position);
inset: var(--b-top) var(--b-right) var(--b-bottom) var(--b-left);
flex-shrink: var(--b-shrink);
flex-grow: var(--b-grow);
background-color: var(--b-bg-color);
border-color: var(--b-border-color);
border-radius: var(--b-border-radius);
border-width: var(--b-border-top-width) var(--b-border-right-width) var(--b-border-bottom-width) var(--b-border-left-width);
border-style: var(--b-border-style);
box-shadow: var(--b-elevation);
z-index: var(--b-z-index);
overflow: var(--b-overflow-x) var(--b-overflow-y);
}
}
4 changes: 2 additions & 2 deletions packages/bezier-react/src/utils/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export const getMarginStyle = <Props extends MarginProps>({
mb,
ml,
}: Props) => ({
'--b-margin-all': cssDimension(m),
'--b-margin': cssDimension(m),
'--b-margin-x': cssDimension(mx),
'--b-margin-y': cssDimension(my),
'--b-margin-top': cssDimension(mt),
Expand Down Expand Up @@ -186,7 +186,7 @@ export const getLayoutStyle = <Props extends LayoutProps>({
overflowX,
overflowY,
}: Props) => ({
'--b-padding-all': cssDimension(p),
'--b-padding': cssDimension(p),
'--b-padding-x': cssDimension(px),
'--b-padding-y': cssDimension(py),
'--b-padding-top': cssDimension(pt),
Expand Down

0 comments on commit ca47057

Please sign in to comment.