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

Panda CSS 의존성 제거 및 기존 코드 수정 #50

Merged
merged 4 commits into from
Nov 5, 2024
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
Binary file modified bun.lockb
Binary file not shown.
5 changes: 1 addition & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,16 @@
"test": "vitest",
"coverage": "vitest run src --coverage",
"storybook": "storybook dev -p 6006",
"build-storybook": "storybook build",
"prepare": "husky & panda codegen"
"build-storybook": "storybook build"
},
"dependencies": {
"@tanstack/react-query": "^5.59.15",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#35 (comment) 를 참고하여 Tanstack Query도 아직 쓰는 곳이 없길래 우선 제거하였습니다.

필요한 분이 나중에 언제든지 추가할 수 있습니다.

"react": "^18.3.1",
"react-dom": "^18.3.1"
},
"devDependencies": {
"@chromatic-com/storybook": "^1.9.0",
"@eslint/js": "^9.13.0",
"@faker-js/faker": "^9.0.3",
"@pandacss/dev": "^0.46.1",
"@storybook/addon-essentials": "^8.3.6",
"@storybook/addon-interactions": "^8.3.6",
"@storybook/addon-links": "^8.3.6",
Expand Down
33 changes: 0 additions & 33 deletions panda.config.ts

This file was deleted.

5 changes: 0 additions & 5 deletions postcss.config.cjs

This file was deleted.

7 changes: 7 additions & 0 deletions src/components/Certificate/Certificate.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.certificate {
footer {
@media print {
display: none;
}
}
}
Comment on lines +1 to +7
Copy link
Contributor Author

@DaleSeo DaleSeo Nov 4, 2024

Choose a reason for hiding this comment

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

CSS Nesting과 같은 Modern CSS 문법을 잘 활용하면 CSS Modules를 쓰더라도 괜찮은 개발자 경험을 할 수 있습니다. 무엇보다 옛날처럼 복잡하게 다수의 스타일을 임포트해서 여러 HTML 요소에 달아주지 않아서 되고요, 타입 선택자를 사용할 수 있기 때문에 일일이 클래스 네이밍을 해줘야하는 스트레스도 줄일 수 있습니다. 혹시 CSS Nesting이 생소하시다면 이 블로그 포스팅을 읽어보시기 바랍니다: https://www.daleseo.com/css-nesting/

12 changes: 4 additions & 8 deletions src/components/Certificate/Certificate.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { css } from "../../../styled-system/css";
import styles from "./Certificate.module.css";

export default function Certificate() {
const member = new URL(location.href).searchParams.get("member");
Expand All @@ -11,21 +11,17 @@ export default function Certificate() {
const linkedInURL = `https://www.linkedin.com/profile/add?startTask=CERTIFICATION_NAME&name=${member}&organizationId=104834174&certUrl=${location.href}`;

return (
<main>
<main className={styles.certificate}>
<section aria-labelledby="certification">
<h2 id="certification">{member}님의 수료증</h2>
<div>
<p>귀하는 어쩌구 저쩌구</p>
</div>
</section>
<section className={invisiblePrint}>
<footer>
<button onClick={() => window.print()}>출력</button>
<a href={linkedInURL}>링크드인에 공유하기</a>
</section>
</footer>
</main>
);
}

const invisiblePrint = css({
"@media print": { display: "none" },
});
9 changes: 9 additions & 0 deletions src/components/Leaderboard/Leaderboard.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
.leaderboard {
article {
ul {
li {
margin-bottom: 20px;
}
}
}
}
2 changes: 1 addition & 1 deletion src/components/Leaderboard/Leaderboard.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe("<Leaderboard/>", () => {

it("renders the members list section", () => {
expect(
screen.getByRole("region", { name: /members list/i }),
screen.getByRole("article", { name: /members list/i }),
).toBeInTheDocument();
});

Expand Down
12 changes: 7 additions & 5 deletions src/components/Leaderboard/Leaderboard.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import styles from "./Leaderboard.module.css";

export default function Leaderboard() {
const members = [
{ name: "DaleSeo", solved: 71, rank: "새싹" },
Expand All @@ -9,14 +11,14 @@ export default function Leaderboard() {
];

return (
<div>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

<Certificate/><Progress/>는 최상위 요소로 <main> 요소를 쓰는데 이 컴포넌트만 <div> 요소를 쓰고 있어서 통일하였습니다.

<main className={styles.leaderboard}>
<h1>Leaderboard</h1>
Copy link
Contributor

@SamTheKorean SamTheKorean Nov 5, 2024

Choose a reason for hiding this comment

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

최상위 요소를 main으로 바꾼다면 #17 (comment) 에서 말씀하시 것처럼 전체 페이지가 main에 들어갈 수 있으니 빼는게 좋을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! 제가 후딱 코드를 고치느라 일관성만 생각하고 더 큰 그림을 보지 못했습니다. 이 부분 나중에 <Layout/> 컴포넌트가 도입될 때 팀 차원에서 다시 한 번 검토하면 좋을 것 같습니다. 지금은 우선 각 페이지별로 <h1> 요소가 있는 정도 만으로 만족해도 될 것 같습니다.


<section aria-labelledby="leaderboard">
<article aria-labelledby="leaderboard">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

각 멤버 카드가 독립적인 컨텐츠이기 때문에 <article> 요소가 더 적합한 semantic 요소라고 판단하였습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

흠 이 부분에 대해서 section으로 할 지 article로 할 지 고민했는데 피드백 감사합니다!

<h2 id="leaderboard">Members List</h2>
<ul>
{members.map((member) => (
<li key={member.name} style={{ marginBottom: "20px" }}>
<li key={member.name}>
<div>등급: {member.rank}</div>
<div>진행 상황: {member.solved}</div>
<div>
Expand All @@ -30,7 +32,7 @@ export default function Leaderboard() {
</li>
))}
</ul>
</section>
</div>
</article>
</main>
);
}
83 changes: 82 additions & 1 deletion src/index.css
Original file line number Diff line number Diff line change
@@ -1 +1,82 @@
@layer reset, base, tokens, recipes, utilities;
:root {
Copy link
Contributor Author

@DaleSeo DaleSeo Nov 4, 2024

Choose a reason for hiding this comment

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

https://github.com/DaleStudy/leetcode-website/blob/main/global-styles.css 의 내용을 그대로 복사해왔습니다. 기존 웹사이트와 일관적인 스타일을 하시는데 도움이 될 거라 생각합니다.

--primary: #846de9;
--secondary: #24eaca;
--bg-100: #fdfdff;
--bg-200: #fbfaff;
--bg-300: #eee8fe;
--bg-400: #5333e1;
--text-900: #160b46;
--font-weight-light: 300;
--font-weight-regular: 400;
--font-weight-medium: 500;
--font-weight-bold: 700;
}

* {
font-family: "Spoqa Han Sans Neo", "Noto Sans KR", sans-serif;
}

*,
*::before,
*::after {
box-sizing: border-box;
}

html {
scroll-behavior: smooth;
}

body {
margin: 0;
}

a {
text-decoration: none;
color: inherit;
background: none;
border: none;
font: inherit;
margin: 0;
padding: 0;
cursor: pointer;
}

a:visited {
color: inherit;
}

a:hover {
color: inherit;
text-decoration: none;
transition: 200ms;
}

a:active {
color: inherit;
}

a:focus {
outline: none;
}

button:hover {
transition: 200ms;
}

h1,
h2,
h3,
h4,
h5,
h6,
ol,
ul,
li {
margin: 0;
padding: 0;
}

ol,
ul {
list-style-type: none;
}
4 changes: 1 addition & 3 deletions tsconfig.app.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
"include": [
"src",
/* Vitest */
"vitest.setup.ts",
/* PandaCSS */
"styled-system"
"vitest.setup.ts"
]
}