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

๐Ÿ”ง ๋ฆฌ๋‚˜๋ฆฌ์•„, ์Šคํ† ๋ฆฌ๋ถ ์„ค์ • #18

Merged
merged 7 commits into from
May 10, 2023

Conversation

leesangb
Copy link
Collaborator

@leesangb leesangb commented May 4, 2023

๋ณ€๊ฒฝ ๋‚ด์šฉ

  • linaria ์ถ”๊ฐ€
  • ์Šคํ† ๋ฆฌ๋ถ 7๋กœ ์—…๊ทธ๋ ˆ์ด๋“œ
  • ๋””์ž์ธ ํ† ํฐ์œผ๋กœ ์ตœ์†Œํ•œ์˜ ํ…Œ๋งˆ ๊ตฌ์„ฑ

์ฒดํฌ๋ฆฌ์ŠคํŠธ

  • ๋ฐฑ์—”๋“œ ์„ ๋ฐฐํฌ
  • Playwright ํ…Œ์ŠคํŠธ ์ผ€์ด์Šค ์ž‘์„ฑ
  • Jest Hook ํ…Œ์ŠคํŠธ ์ผ€์ด์Šค ์ž‘์„ฑ
  • Storybook ํ…Œ์ŠคํŠธ ์ผ€์ด์Šค ์ž‘์„ฑ

๐Ÿ“ธ ์Šคํฌ๋ฆฐ์ƒท

๊ธฐํƒ€์‚ฌํ•ญ


๐Ÿคš๐Ÿป ์ฝ”๋“œ๋ฆฌ๋ทฐ ํ•˜๋Š” ๋ฐฉ๋ฒ•

๐Ÿชง [๊ฐœ๋ฐœ ๊ณตํ†ต ์ฝ”๋“œ๋ฆฌ๋ทฐ ๊ฐ€์ด๋“œ]

โœŽ Code Review Manner

  1. ์ง€์ ์ด ์•„๋‹Œ ์งˆ๋ฌธํ•˜๊ธฐ
    • ์›์ž‘ํ•˜์‹ ๋ถ„์ด ๋ชฐ๋ผ์„œ ๊ทธ๋ ‡๊ฒŒ ํ•œ๊ฒŒ ์•„๋‹ˆ๋ผ ๊ทธ๋žฌ์„ ์ˆ˜ ๋ฐ–์— ์—†๋Š” ์ƒํ™ฉ์ผ ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค
    • ์œ„ํ—˜ํ•˜๋‹ค๊ณ  ํŒ๋‹จ์‹œ request change
  2. ๋‹ค๋ฅธ ๋ฐฉ๋ฒ•์— ๋Œ€ํ•ด ์ œ์•ˆํ•˜๊ธฐ
    • ์ด ๋•Œ ๋ณธ์ธ์ด ์ƒ๊ฐํ•œ ๊ฒƒ๊ณผ ๋„ˆ๋ฌด ๋‹ค๋ฅธ ๊ฒฝ์šฐ request change
  3. ์˜ˆ์‹œ์ฝ”๋“œ๋ฅผ ์ž‘์„ฑํ•˜์—ฌ ๋Œ“๊ธ€๋กœ ๋‹ฌ๊ธฐ
  4. ๋ฆฌ๋ทฐ ๋งˆ๋ฌด๋ฆฌ
    • ๋ฌดํ”Œ ๋ฐฉ์ง€! ๊ณ ์ƒํ•œ ์‚ฌ์šฐ์—๊ฒŒ ๊ณ ์ƒํ–ˆ๋‹ค ํ•œ๋งˆ๋”” ๋‚จ๊ธฐ๋Š” ๊ฒŒ ์ข‹์Šต๋‹ˆ๋‹ค (LGTM ๊ฐ•์ถ”)

โœŽ Pn Rule

  • Pn

    • P1 : ๊ผญ ๋ฐ˜์˜ํ•ด์ฃผ์„ธ์š” (Request changes)
    • P2 : ์ ๊ทน์ ์œผ๋กœ ๊ณ ๋ คํ•ด์ฃผ์„ธ์š” (Request changes)
    • P3 : ์›ฌ๋งŒํ•˜๋ฉด ๋ฐ˜์˜ํ•ด ์ฃผ์„ธ์š” (Comment)
    • P4 : ๋ฐ˜์˜ํ•ด๋„ ์ข‹๊ณ  ๋„˜์–ด๊ฐ€๋„ ์ข‹์Šต๋‹ˆ๋‹ค (Approve)
    • P5 : ๊ทธ๋ƒฅ ์‚ฌ์†Œํ•œ ์˜๊ฒฌ์ž…๋‹ˆ๋‹ค (Approve)
  • Pn Resolve

    • P1, P2, P3๋Š” ๋ฆฌ๋ทฐ์–ด๊ฐ€ Resolve ํ•ฉ๋‹ˆ๋‹ค
    • P4, P5๋Š” ๋ฆฌ๋ทฐ์–ด๊ฐ€ ์•„๋‹ˆ๋”๋ผ๋„ Resolve ํ•  ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค
    • Pn ํ‘œ์‹œ๊ฐ€ ์—†์„ ๊ฒฝ์šฐ P5๋กœ ๋ด…๋‹ˆ๋‹ค

โœŽ PR Title Rule

  • PR์— ๋ฐฐํฌ์ผ ํ‘œ๊ธฐ๊ฐ€ ํ•„์š”ํ•œ ๊ฒฝ์šฐ
      # <gitmoji> PR ์ œ๋ชฉ
    

โœŽ Comment Rule

  • Comment์˜ Resolve ์ฒ˜๋ฆฌ
    • ์ตœ์ดˆ Comment๋ฅผ ์ž‘์„ฑํ•œ ์‚ฌ๋žŒ์ด ์ฒ˜๋ฆฌ

โœŽ Merge Rule

  • PR ๋จธ์ง€๋Š” ๋ฆฌ๋ทฐ์–ด๊ฐ€ ํ•ด์ฃผ๋„๋ก ํ•ฉ๋‹ˆ๋‹ค.

@leesangb leesangb added the pending ๋กœ์ปฌ ๊ฐœ๋ฐœ ์ง„ํ–‰์ค‘ ์ž…๋‹ˆ๋‹ค. label May 4, 2023
import type { StorybookConfig } from "@storybook/nextjs";
import MiniCssExtractPlugin, { Configuration } from "mini-css-extract-plugin";

const withLinaria = (config: Configuration) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

๋ฆฌ๋‚˜๋ฆฌ์•„์šฉ ์›นํŒฉ ์„ค์ •ํ•˜๋Š” ๋ถ€๋ถ„์ž…๋‹ˆ๋‹ค
๋ฐ”๋ฒจ๋กœ๋”์™€ ๋ฆฌ๋‚˜๋ฆฌ์•„ ์›นํŒฉ๋กœ๋” ๋ชจ๋‘ ์„ค์ •ํ•ด์ค˜์•ผํ•˜๊ณ  css ๋กœ๋”๋„ ์„ค์ •ํ•ด์ค˜์•ผ ํ•ฉ๋‹ˆ๋‹ค

Copy link
Collaborator

Choose a reason for hiding this comment

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

๋ฆฌ๋‚˜๋ฆฌ์•„ ์Šคํ† ๋ฆฌ๋ถ ์„ค์ •์ด ๋ณต์žกํ•˜๋„ค์—ฌ ๊ณ ์ƒํ•˜์…จ์Šต๋‹ˆ๋‹ค ๐Ÿ™‡โ€โ™‚๏ธ

(Story) => {
const isDark = useDarkMode();

useEffect(() => {
Copy link
Collaborator Author

@leesangb leesangb May 4, 2023

Choose a reason for hiding this comment

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

์Šคํ† ๋ฆฌ๋ถ์—์„œ ๋‹คํฌ/๋ผ์ดํŠธ๋ชจ๋“œ ๋ณ€๊ฒฝํ•˜๋Š” ๋ถ€๋ถ„ ์ž…๋‹ˆ๋‹ค
image

์Šคํ† ๋ฆฌ๋ถ ์ž์ฒด ์ƒ‰์ƒ์ด ๋ณ€๊ฒฝ๋˜์ง€ ์•Š๋Š”๊ฑด ์Šคํ† ๋ฆฌ๋ถ 7์— ์ด์Šˆ๊ฐ€ ์žˆ๋‹ค๊ณ  ํ•˜๋„ค์š”
์ฐธ๊ณ : storybookjs/storybook#21798

Copy link
Collaborator

Choose a reason for hiding this comment

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

ใ…‡ใ…Ž.. ํ…Œ๋งˆ ๋ณ€๊ฒฝ ๋Œ€์‘๋„ ํ•ด์ฃผ์…”์„œ ๊ฐ์‚ฌํ•ฉ๋‹ˆ๋‹ค.

"use client";

// FIXME: temporary solution
export const ThemeSwitcher = () => {
Copy link
Collaborator Author

@leesangb leesangb May 4, 2023

Choose a reason for hiding this comment

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

ํ…Œ๋งˆ ํ† ๊ธ€ ํ•˜๋Š” ์ž„์‹œ์šฉ ์ปดํฌ๋„ŒํŠธ ์ž…๋‹ˆ๋‹ค. layout์— ํ…Œ์ŠคํŠธ ์šฉ๋„๋กœ ์ถ”๊ฐ€ํ–ˆ์Šต๋‹ˆ๋‹ค. ์ถ”ํ›„์— ์‚ญ์ œ ํ•  ์˜ˆ์ •์ž…๋‹ˆ๋‹ค.
image

src/components/button/Button.stories.tsx Show resolved Hide resolved
import { commonThemeVar, darkThemeVar, lightThemeVar, theme } from "@/mds/theme";
import { css } from "@linaria/core";

export const globals = css`
Copy link
Collaborator Author

@leesangb leesangb May 4, 2023

Choose a reason for hiding this comment

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

linaria์˜ global css์ž…๋‹ˆ๋‹ค.
next-with-linaria ํŒจํ‚ค์ง€์—์„œ ๋„ค์ด๋ฐ ์ปจ๋ฒค์…˜์ด ์žˆ๊ธฐ ๋•Œ๋ฌธ์— ํŒŒ์ผ๋ช…์„ ์ด๋ ‡๊ฒŒ ์ง€์—ˆ์Šต๋‹ˆ๋‹ค

์ฐธ๊ณ : https://github.com/dlehmhus/next-with-linaria#global-styles-restrictions

Copy link
Collaborator

Choose a reason for hiding this comment

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

ํ…Œ๋งˆ๋ฅผ className์œผ๋กœ ๊ตฌ๋ถ„ํ•˜๋Š” ๊ตฐ์—ฌ ๐Ÿ˜…


//region Types

interface ModePalette {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

๋””์ž์ธ ์‹œ์Šคํ…œ์ด ์—†์–ด ์ž„์‹œ๋กœ ๋งŒ๋“  ๋ผ์ดํŠธ/๋‹คํฌ ๋ชจ๋“œ ํŒ”๋ ˆํŠธ ํƒ€์ž…์ž…๋‹ˆ๋‹ค

//region ThemeVar
export const lightThemeVar = toCssVar({ palette: light });
export const darkThemeVar = toCssVar({ palette: dark });
export const commonThemeVar = toCssVar({ palette: common, font: { family: global.fontFamily.KR.value } });
Copy link
Collaborator Author

@leesangb leesangb May 4, 2023

Choose a reason for hiding this comment

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

ํ•ด๋‹น ๊ฐ’๋“ค์„ ๊ธ€๋กœ๋ฒŒ css์— ์ถ”๊ฐ€ํ•˜๋ฉด ์ด๋Ÿฐ์‹์œผ๋กœ ๋‚˜์˜ต๋‹ˆ๋‹ค

image

@@ -18,6 +18,7 @@
"isolatedModules": true,
"jsx": "preserve",
"incremental": true,
"baseUrl": ".",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

baseUrl์ด ๋น ์ ธ์žˆ์–ด์„œ @/xxx importํ•  ๋•Œ ๋ฌธ์ œ๊ฐ€ ๋์—ˆ์Šต๋‹ˆ๋‹ค.

}
};

export const theme: Theme = (<T extends object>({
Copy link
Collaborator Author

@leesangb leesangb May 4, 2023

Choose a reason for hiding this comment

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

styled๋กœ ์ปดํฌ๋„ŒํŠธ ๋งŒ๋“ค ๋•Œ ์“ธ ํ…Œ๋งˆ ๊ฐ์ฒด์ž…๋‹ˆ๋‹ค
image

};
}

type ColorKey = "100" | "200" | "300" | "400" | "500" | "600" | "700" | "800" | "900";
Copy link
Collaborator Author

@leesangb leesangb May 4, 2023

Choose a reason for hiding this comment

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

  • ๊ทธ๋ ˆ์ด ์ƒ‰์ƒ์€ 50, 75, 1000๋„ ์žˆ์Šต๋‹ˆ๋‹ค

Copy link
Collaborator

@Doodream Doodream left a comment

Choose a reason for hiding this comment

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

๐Ÿ‘ ๊ณ ์ƒํ•˜์…จ์Šต๋‹ˆ๋‹น

import type { StorybookConfig } from "@storybook/nextjs";
import MiniCssExtractPlugin, { Configuration } from "mini-css-extract-plugin";

const withLinaria = (config: Configuration) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

๋ฆฌ๋‚˜๋ฆฌ์•„ ์Šคํ† ๋ฆฌ๋ถ ์„ค์ •์ด ๋ณต์žกํ•˜๋„ค์—ฌ ๊ณ ์ƒํ•˜์…จ์Šต๋‹ˆ๋‹ค ๐Ÿ™‡โ€โ™‚๏ธ

(Story) => {
const isDark = useDarkMode();

useEffect(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ใ…‡ใ…Ž.. ํ…Œ๋งˆ ๋ณ€๊ฒฝ ๋Œ€์‘๋„ ํ•ด์ฃผ์…”์„œ ๊ฐ์‚ฌํ•ฉ๋‹ˆ๋‹ค.

import { commonThemeVar, darkThemeVar, lightThemeVar, theme } from "@/mds/theme";
import { css } from "@linaria/core";

export const globals = css`
Copy link
Collaborator

Choose a reason for hiding this comment

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

ํ…Œ๋งˆ๋ฅผ className์œผ๋กœ ๊ตฌ๋ถ„ํ•˜๋Š” ๊ตฐ์—ฌ ๐Ÿ˜…

});
};

flattenRec(obj, "");
Copy link
Collaborator

Choose a reason for hiding this comment

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

prefix๊ฐ€ ๋ฌด์กฐ๊ฑด ""์ธ๋ฐ flattenRec์— prefix ์ธ์ˆ˜๋ฅผ ๋งŒ๋“ ์ด์œ ๊ฐ€ ์žˆ์„๊นŒ์š”?

Copy link
Member

Choose a reason for hiding this comment

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

์ด์ „ ์ €์žฅ์†Œ์— ํ•ด๋‹น ์ฝ”๋“œ์— ๋Œ€ํ•œ ์ฃผ์„์ด ์žˆ๋Š”๋ฐ, ๊ฐ์ฒด๋ฅผ ํŽผ์น˜๋Š” ๊ณผ์ •์—์„œ ์ดˆ๊ธฐ์—๋งŒ ๋นˆ ๋ฌธ์ž์—ด์„ ๋„ฃ๋Š” ๊ฒƒ ๊ฐ™์•„์š”!

์ดํ›„์—๋Š” ์žฌ๊ท€๋กœ ๋Œ๋ฉด์„œ ๋‹ค์ค‘ ๊ฐ์ฒด์˜ key ๊ฐ’๋“ค์„ ๋„ฃ๋Š” ๋“ฏ ํ•ฉ๋‹ˆ๋‹ค.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

๋„ต๋„ต ๋‚˜๋ฌด๋‹˜์ด ๋ง์”€ํ•ด์ฃผ์‹ ๋Œ€๋กœ ์žฌ๊ท€๋กœ ์ฑ„์›Œ์ง€๋Š” ๋ฌธ์ž์—ด์ž…๋‹ˆ๋‹ค

)
}),
{}
) as Colors
Copy link
Collaborator

Choose a reason for hiding this comment

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

p5 ์Œ .. global.json ํŒŒ์ผ์„ ๊ฐ์ฒด๋กœ ๋ณ€ํ™˜ํ•ด์„œ ts ํŒŒ์ผ๋กœ ๋งŒ๋“œ๋Š” ์Šคํฌ๋ฆฝํŠธ๋ฅผ ๋Œ๋ฆฌ๊ณ 
ํ•ด๋‹น ๊ฐ์ฒด๋ฅผ ๊ธฐ๋ฐ˜์œผ๋กœ as const ํ•ด์„œ readable ํƒ€์ž…์„ ๋งŒ๋“ค์–ด ์จ์„œ global ํ† ํฐ ํƒ€์ž…์„ ๋งŒ๋“œ๋Š” ๊ฑด ์–ด๋–จ๊นŒ์š”?
๊ทธ๋Ÿผ ์ €๋ ‡๊ฒŒ Colors ๋ผ๊ณ  ๋ถ€์ •ํ™•ํ•œ ํƒ€์ž…์„ aliasํ•˜์ง€ ์•Š์•„๋„ ๋  ๊ฒƒ ๊ฐ™์•„์š”.

Copy link
Member

Choose a reason for hiding this comment

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

Colors ํƒ€์ž… ์ž์ฒด๊ฐ€ global.color์˜ key๊ฐ’์„ ํƒ€์ž…์œผ๋กœ ์ฝ์–ด์˜ค๋Š”๋ฐ ๊ดœ์ฐฎ์ง€ ์•Š์„๊นŒ์š”?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Doodream ๊ทธ๋Ÿฌ๋ฉด style dictionary๋ฅผ ์จ์„œ json์„ ts๋กœ ๋ณ€ํ™˜ํ•œ๋‹ค๋Š”๊ฑธ ๋ง์”€ํ•˜์‹ ๊ฑด๊ฐ€์š”?
as Colors๋กœ ์‚ฌ์šฉํ•˜๋Š”๊ฑด ์ €๋„ ์กฐ๊ธˆ ๊ฑธ๋ฆฌ๊ธดํ•ฉ๋‹ˆ๋‹ค... reduce๋กœ ํƒ€์ž… ์ถ”๋ก ์„ ์ œ๋Œ€๋กœ ๋ชปํ•ด์„œ ๋งŒ๋“ค์–ด์ง„ ํƒ€์ž…์€ Record<string, Record<ColorKey, string>>์ด์ง€๋งŒ ์‹ค์ œ๋กœ๋Š” ์ € ํƒ€์ž…์ด ๋งž์•„์š”... ์ž„์‹œ๋กœ ์ด๋ ‡๊ฒŒ ํ•ด๋‘๊ณ  ๋” ์ข‹์€ ๋ฐฉ๋ฒ•์ด ์žˆ๋‹ค๋ฉด ๊ทธ๋•Œ ๋ณ€๊ฒฝํ•˜๊ฒ ์Šต๋‹ˆ๋‹ค

Copy link
Member

@ixio0330 ixio0330 left a comment

Choose a reason for hiding this comment

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

๋ฆฌ๋‚˜๋ฆฌ์•„ ์„ธํŒ…, ์Šคํ† ๋ฆฌ๋ถ ์„ค์ •์— ๋””์ž์ธ ์‹œ์Šคํ…œ ์ ์šฉ๊นŒ์ง€,, ์ •๋ง ๊ณ ์ƒ ๋งŽ์œผ์…จ์Šต๋‹ˆ๋‹ค. ๐Ÿ™

@leesangb leesangb merged commit 8e2c869 into main May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending ๋กœ์ปฌ ๊ฐœ๋ฐœ ์ง„ํ–‰์ค‘ ์ž…๋‹ˆ๋‹ค.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants