Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ui-kit): Row 컴포넌트 추가 및 그리드 인터페이스 확정 #27

Merged
merged 5 commits into from
Dec 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 6 additions & 18 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,10 @@
<!-- 절취선 -->
# 이 Pull Request가 BREAKING CHANGE를 포함하고 있나요?
BREAKING CHANGE가 포함된 PR이라면 Base를 Beta로 잡아주셔야 합니다 !
확인 후 이 내용은 지워주셔도 됩니다.
<!-- 절취선 -->
> 이 PR이 BREAKING_CHANGE를 포함하고 있다면 반드시 명시해주세요!
## 주요 변경사항
<!-- 이 PR이 해결하는 문제 -->
## 변경사항
어떤 사항이 변경되었는지 자세히 적어주세요!

## 링크
- 디자인 시안 링크: <!-- 제플린, 피그마, 프레이머 등 -->
- 슬랙 링크: <!-- 관련된 대화가 이루어진 슬랙 링크 -->
## 디자인 시안 링크
디자인 시안 링크를 첨부해주세요!

## 시급한 정도
<!-- 아래에서 선택해주세요. -->
---
⚠︎ 긴급 : 선 어프루브 후 리뷰를 부탁드립니다. (리뷰는 추후 반영)
🏃‍♂️ 보통 : 최대한 빠르게 리뷰 부탁드립니다.
🐢 천천히 : 급하지 않습니다.
## 집중적으로 리뷰 받고 싶은 부분이 있나요?

## 중점적으로 봐주었으면 하는 부분
<!-- 변경사항이 큰 경우 집중해야 할 부분, 또는 불안해서 봐주었으면 하는 부분 등 -->
19 changes: 0 additions & 19 deletions ui-kit/src/components/Column/index.stories.tsx

This file was deleted.

57 changes: 0 additions & 57 deletions ui-kit/src/components/Column/index.tsx

This file was deleted.

1 change: 0 additions & 1 deletion ui-kit/src/components/Column/types.ts

This file was deleted.

38 changes: 38 additions & 0 deletions ui-kit/src/components/Grid/Column.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import React, { ElementType, useMemo } from 'react';
import { ColumnSize, ColumnResponsive, DEFAULT_ELEMENT } from './types';
import { OverridableProps } from 'types/OverridableProps';
import classNames from 'classnames';

const sizes: ColumnResponsive[] = ['xl', 'lg', 'md', 'sm', 'xs'];

type ColumnBaseProps = {
[key in ColumnResponsive]?: ColumnSize;
};

type ColumnProps<T extends ElementType = typeof DEFAULT_ELEMENT> = OverridableProps<
T,
ColumnBaseProps
>;

const Column = <T extends React.ElementType = typeof DEFAULT_ELEMENT>(
{ as, ...props }: ColumnProps<T>,
ref: React.Ref<any>
) => {
const spanClasses = useMemo(
() =>
sizes.map((size) => {
const { [size]: sizeValue } = props;
return sizeValue ? `lubycon-grid-column--${size}__${sizeValue}` : '';
}),
[]
);
Comment on lines +21 to +28
Copy link
Member Author

Choose a reason for hiding this comment

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

만약 사용자가 xsmd 같은 사이즈 프로퍼티를 사용할 때만 lubycon-grid--column--${size}__${sizeValue} 클래스를 바인딩하도록 변경하였습니다. 만약 사이즈 프로퍼티가 없다면 컬럼은 기본적인 flex 내부 요소처럼 동작합니다.


const target = as ?? DEFAULT_ELEMENT;
const Component = target;

return (
<Component ref={ref} className={classNames(`lubycon-grid-column`, ...spanClasses)} {...props} />
);
};

export default React.forwardRef(Column) as typeof Column;
34 changes: 34 additions & 0 deletions ui-kit/src/components/Grid/Row.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import React, { ElementType, Ref, forwardRef } from 'react';
import { DEFAULT_ELEMENT } from './types';
import classnames from 'classnames';
import { OverridableProps } from 'src/types/OverridableProps';

type BaseAlign = 'flex-start' | 'center' | 'flex-end';
interface RowBaseProps {
direction?: 'column' | 'row' | 'row-reverse' | 'column-reverse';
Copy link
Member Author

Choose a reason for hiding this comment

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

이 프로퍼티는 Mateiral UI를 참고해서 그대로 구현했습니다. 사실 column이나 column-reverse 일 때는 컬럼의 사이즈 프로퍼티가 의미가 없기는 한데, 확장성 때문에 요렇게 설계한 것 같아유.

https://material-ui.com/components/grid/#interactive

justify?: BaseAlign | 'space-between' | 'space-around' | 'space-evenly';
alignItems?: BaseAlign | 'stretch' | 'baseline';
Comment on lines +8 to +10
Copy link
Member Author

Choose a reason for hiding this comment

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

혹시 더 필요한 옵션이 있을까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

BaseAlign이랑 이 옵션에서 벗어나는 경우를 못봐서 저는 괜찮다고 생각합니다

Copy link
Contributor

Choose a reason for hiding this comment

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

flex shrink, basis, grow 는 필요 없을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

그리드가 어떻게 생겼는지 디자인 적으로 정해져있는 상태에서 shrink, basis, grow를 사용해버리면 그리드 레이아웃이 깨지게 되는데, 그럼 그리드를 사용하는 것이 의미가 없게 되어버릴 것 같다는 생각입니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

가끔 shrink 나 grow 를 쓰긴하는데 그리드 레이아웃이 깨지는거 생각하면 아직은 도입 안해도 될것 같은 옵션이네욤 그러면 저 위의 옵션으로 괜찮을거 같아요

}
type RowProps<T extends ElementType = typeof DEFAULT_ELEMENT> = OverridableProps<T, RowBaseProps>;

const Row = (
{ as, direction = 'row', justify = 'flex-start', alignItems = 'flex-start', ...props }: RowProps,
ref: Ref<any>
) => {
const Component = as ?? DEFAULT_ELEMENT;

return (
<Component
ref={ref}
className={classnames(
'lubycon-grid-row',
`lubycon-grid-row--direction__${direction}`,
`lubycon-grid-row--justify__${justify}`,
`lubycon-grid-row--align-items__${alignItems}`
)}
{...props}
/>
);
};

export default forwardRef(Row) as typeof Row;
2 changes: 2 additions & 0 deletions ui-kit/src/components/Grid/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { default as Row } from './Row';
export { default as Column } from './Column';
5 changes: 5 additions & 0 deletions ui-kit/src/components/Grid/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const DEFAULT_ELEMENT = 'div' as const;

type ColumnNumberSize = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12;
export type ColumnSize = boolean | 'auto' | ColumnNumberSize;
export type ColumnResponsive = 'xl' | 'lg' | 'md' | 'sm' | 'xs';
11 changes: 0 additions & 11 deletions ui-kit/src/components/Row/index.tsx

This file was deleted.

Empty file.
2 changes: 1 addition & 1 deletion ui-kit/src/components/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export { default as Button } from './Button';
export { default as Text } from './Text';
export { default as Column } from './Column';
export { Row, Column } from './Grid';
75 changes: 11 additions & 64 deletions ui-kit/src/sass/components/_Column.scss
Original file line number Diff line number Diff line change
@@ -1,70 +1,17 @@
$grid-breakpoints: (
xs: 0,
sm: 576px,
md: 768px,
lg: 992px,
xl: 1200px,
) !default;

@function breakpoint-min($name, $breakpoints: $grid-breakpoints) {
$min: map-get($breakpoints, $name);
@return if($min != 0, $min, null);
}

@function breakpoint-infix($name, $breakpoints: $grid-breakpoints) {
@return if(breakpoint-min($name, $breakpoints) == null, '', '-#{$name}');
}

@mixin media-breakpoint-up($name, $breakpoints: $grid-breakpoints) {
$min: breakpoint-min($name, $breakpoints);
@if $min {
@media (min-width: $min) {
@content;
}
} @else {
@content;
}
.lubycon-grid-column {
position: relative;
width: 100%;
padding-right: ($gutter / 2);
padding-left: ($gutter / 2);
}

@mixin make-column($size, $columns: $grid-columns) {
flex: 0 0 auto;
width: percentage($size / $columns);
}

@mixin generate-grid-columns($columns: 12, $gutter: 12px, $breakpoints: $grid-breakpoints) {
%grid-column {
position: relative;
width: 100%;
padding-right: ($gutter / 2);
padding-left: ($gutter / 2);
}

@each $breakpoint in map-keys($breakpoints) {
$infix: breakpoint-infix($breakpoint, $breakpoints);

@for $i from 1 through $columns {
.column#{$infix}-#{$i} {
@extend %grid-column;
}
}
.column#{$infix} {
@extend %grid-column;
}

@include media-breakpoint-up($breakpoint, $breakpoints) {
.column#{$infix} {
flex-basis: 0;
flex-grow: 1;
max-width: 100%;
}

@for $i from 1 through $columns {
.column#{$infix}-#{$i} {
@include make-column($i, $columns);
}
@each $breakpoint, $size in $breakpoints {
@for $i from 1 through $max-columns {
.lubycon-grid-column--#{$breakpoint}__#{$i} {
@include media-breakpoint($size) {
flex: 0 0 auto;
width: percentage($i / $max-columns);
}
}
}
}

@include generate-grid-columns();
39 changes: 39 additions & 0 deletions ui-kit/src/sass/components/_Row.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
@mixin direction($direction) {
&--direction__#{$direction} {
Copy link
Member Author

Choose a reason for hiding this comment

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

이게 쓸 때마다 마음에 걸리는 부분인데요.

다들 아시겠지만...이 클래스 네임이 컴포넌트 내부에서 클래스를 렌더하는 로직과 종속이 걸려있슴다.

<Component
      ref={ref}
      className={classnames(
        'lubycon-grid-row',
        `lubycon-grid-row--direction__${direction}`,
        `lubycon-grid-row--justify__${justify}`,
        `lubycon-grid-row--align-items__${alignItems}`
      )}
      {...props}
    />

그래서 컴포넌트 내에서 클래스 네임을 수정했지만 CSS 내부의 선택자가 수정하지 않으면 바로 버그로 이어집니다. 근데 사실 소스 까보기 전까진 이런 종속이 있다는 것을 파악하기가 어려워서 조금 위험한 패턴인 것 같기도 해용...

금주 챕터 미팅 때 한번 요거 논의해보면 좋을 것 같기도 합니다..!

flex-direction: $direction;
}
}
@mixin justify($align) {
&--justify__#{$align} {
justify-content: $align;
}
}
@mixin alignItems($align) {
&--align-items__#{$align} {
align-items: $align;
}
}

.lubycon-grid-row {
display: flex;
width: 100%;
margin: #{-$gutter / 2};
Copy link
Member Author

Choose a reason for hiding this comment

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

로우의 맨 오른쪽과 왼쪽은 거터가 없어야해서, 마진을 음수로 줘서 해결합니당 (부트스트랩 따라함)


@include direction(row);
@include direction(column);
@include direction(row-reverse);
@include direction(column-reverse);

@include justify(flex-start);
@include justify(center);
@include justify(flex-end);
@include justify(space-between);
@include justify(space-around);
@include justify(space-evenly);

@include alignItems(flex-start);
@include alignItems(center);
@include alignItems(flex-end);
@include alignItems(stretch);
@include alignItems(baseline);
}
1 change: 1 addition & 0 deletions ui-kit/src/sass/components/_index.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
@import './Button';
@import './Text';
@import './Radio';
@import './Row';
@import './Column';
13 changes: 13 additions & 0 deletions ui-kit/src/sass/utils/_breakpoints.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
$breakpoints: (
xs: 0,
sm: 576px,
md: 768px,
lg: 992px,
xl: 1200px,
) !default;

@mixin media-breakpoint($size) {
@media (min-width: $size) {
@content;
}
}
2 changes: 2 additions & 0 deletions ui-kit/src/sass/utils/_grid.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
$max-columns: 12;
$gutter: 12px;
2 changes: 2 additions & 0 deletions ui-kit/src/sass/utils/_index.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@import './breakpoints';
@import './grid';
@import './font';
@import './font-weights';
@import './typography';
Expand Down
Loading