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

[Hemdi] FlexBox gap rem 단위, 속성 추가 #11

Merged
merged 3 commits into from
Dec 1, 2022
Merged

Conversation

hemudi
Copy link
Contributor

@hemudi hemudi commented Nov 30, 2022

close #10

✨ 구현 내역

1. gap rem 단위 추가

export interface FlexBoxSX extends SizeSX, SpacingSX, StyleSX {
   ...
  gap?: SpacingValue;
}

const getFlexCssProperties = (sx: FlexBoxSX, theme: DefaultTheme) =>
  Object.entries(sx).reduce((css, [key, value]) => {
    switch (true) {
      case key === 'gap':
        return {
          ...css,
          gap: addSpacingUnit(value),  // typeof value === 'string' ? value : `${value}rem`
        };
      default:
        return { ...css, [key]: value };
    }
  }, {});

기존에 gap에다가 그냥 SpacingValue 타입 지정만 해놓고 단위를 추가해주는 로직을 구현해놓지 않아서 rem이 적용이 안되는 문제를 제이미께서 제보해주셨습니다! 그 부분을 sapcing.ts에 addSpacingUnit 이라는 함수를 구현해 추가했습니다.
함수로 굳이 적용한 이유는 gap이 현재 spacing의 type에 의존되어 있어서 추후에 spacing에 변화가 있을 때 유지보수하기 쉬울 것 같아서 함수로 분리해서 적용했습니다!

2. flex box 속성 추가

export interface FlexBoxSX extends SizeSX, SpacingSX, StyleSX {
  ...
  flexWrap?: 'nowrap' | 'wrap' | 'wrap-reverse';
  flexShrink?: number;
  flexGrow?: number;
}

페이지 마크업을 하다가 FlexBox에 Wrap, Shrink, Grow 속성이 필요할 것 같아 추가하게 되었습니다.

그런데 하나 고민인건 shrink랑 grow는 Flex 아이템에 적용되는 속성인데 지금처럼 FlexBox에 추가해서 함께 써도 괜찮은가? 따로 FlexItem을 구분해야하나? 라는 고민이 드네요!

@netlify
Copy link

netlify bot commented Nov 30, 2022

Deploy Preview for idyllic-puffpuff-476e23 ready!

Name Link
🔨 Latest commit 24ad177
🔍 Latest deploy log https://app.netlify.com/sites/idyllic-puffpuff-476e23/deploys/63879cf32a516c00092ab5d7
😎 Deploy Preview https://deploy-preview-11--idyllic-puffpuff-476e23.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@healtheloper healtheloper left a comment

Choose a reason for hiding this comment

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

고생했습니다 햄디~

shrink랑 grow는 Flex 아이템에 적용되는 속성인데 지금처럼 FlexBox에 추가해서 함께 써도 괜찮은가? 따로 FlexItem을 구분해야하나? 라는 고민이 드네요!

필요로 하는 곳에서 FlexBox 를 상위 컨테이너로 합성하여 쓰면 되지 않을까요?

Copy link
Member

@mina-gwak mina-gwak left a comment

Choose a reason for hiding this comment

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

그런데 하나 고민인건 shrink랑 grow는 Flex 아이템에 적용되는 속성인데 지금처럼 FlexBox에 추가해서 함께 써도 괜찮은가? 따로 FlexItem을 구분해야하나? 라는 고민이 드네요!

저는 FlexBox가 중첩되서 쓰일 경우에 FlexBox 자체가 자식 요소가 될 수 있으니 괜찮을 것 같다고 생각해요!
고생하셨습니다 햄디! 👏

@hemudi hemudi merged commit 8a106ff into dev Dec 1, 2022
@hemudi hemudi deleted the feat/Flexbox/#10 branch December 1, 2022 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FlexBox gap rem 단위, 속성 추가
3 participants