Skip to content

refactor: 스타일 통합 진행 - #13

Draft
jhlee0409 wants to merge 3 commits into
socialdevclub:mainfrom
jhlee0409:feat/integration-style
Draft

refactor: 스타일 통합 진행#13
jhlee0409 wants to merge 3 commits into
socialdevclub:mainfrom
jhlee0409:feat/integration-style

Conversation

@jhlee0409

@jhlee0409 jhlee0409 commented Jan 6, 2025

Copy link
Copy Markdown
Member
  • 현재 emotion + linaria 기반으로 되어있던 컴포넌트들을 emotion으로 통합
  • 말이 통합이지 진짜 라이브러리 교체만 한 수준이라, 리팩토링 수준에서의 작업은 미진행
  • PR 머지 이후 UI kit 재설계 제의 후 -> 통과 시 진행 예정

@jhlee0409
jhlee0409 marked this pull request as draft January 6, 2025 12:59
return (
<Radix.Root type="always" className={scrollViewStyle} data-f="SR-5f71" {...props}>
<Radix.Viewport className={viewportStyle} onScroll={onScroll} data-f="SV-3fb5">
<RootContainer type="always" data-f="SR-5f71" {...props}>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Radix 라이브러리를 참조하고 있다는 것을 명확하게 나타내는 것이 좋다고 생각하기에
이전 그대로 두는 것은 어떤가요?

@jhlee0409 jhlee0409 Jan 6, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

저는 UI kit의 역할을 하는 컴포넌트의 경우 본질적으로 구현부를 알 필요가 없다고 판단하기도 했고,
관리 포인트를 하나로 묶는 것이 좋다라고 생각해서 별도의 컴포넌트로 나누었습니다만 하이안의 의견을 반영해서 좀 더 찾아보니

import { css } from '@emotion/react'

const cardCss = {
  self: css({
    backgroundColor: 'white',
    border: '1px solid #eee',
    borderRadius: '0.5rem',
    padding: '1rem'
  }),

  title: css({
    fontSize: '1.25rem'
  })
}

export function Card({ title, children }) {
  return (
    <div css={cardCss.self}>
      <h5 css={cardCss.title}>{title}</h5>
      {children}
    </div>
  )
}

이런 식으로 radix의 참조를 직관적으로 표기하되, css에 대한 관리체계를 더 명확하게 할 수 있을것 같네요.
헤드리스의 포인트와, 실질적일 스타일링의 분리도 하나의 장점이 될 수 있을 것 같습니다.

두 가지 관점모두 장단점이 있을 것 같습니다

한번 참고해서 수정해보고 다시 리뷰 요청드리겠습니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants