[Feature] Grid 페이지, Button 페이지 마크업 #184
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hamo-o
requested review from
eugene028,
SeieunYoo and
soulchicken
as code owners
|
|
Size Change: 0 B Total Size: 680 kB ℹ️ View Unchanged
|
eugene028
approved these changes
Dec 30, 2024
apps/wow-docs/public/grid/grid-3.png
Outdated
There was a problem hiding this comment.
P3;
전체적으로 png로 이미지를 저장해주셨는데, 투명 배경을 사용하지 않는 파일은, jpg 파일 형식으로 저장하는게 더 좋을 것 같아요!
일반적으로 jpg 파일이 png 파일보다 용량이 작아사 이미지가 많은 저희 사이트에서 성능을 조금이라도 올리려면 요런 부분에 챙겨보면 좋을 거 같아요!
There was a problem hiding this comment.
이전 회의에서 언급했던 것처럼 next/image의 .webp 확장자 변환으로 크게 상관 없을 것 같습니다..!
다만 화질 개선을 위해 논의되었듯이 pdf -> png 또는 pdf -> jpg 변환을 시도했는데 변환 시 또다시 깨지는 현상이 발생하여... 더 알아보고 한꺼번에 개선해야 할 것 같습니다.
There was a problem hiding this comment.
4x png 와 svg 모두 뽑아서 비교해봤는데 화질이나 크기 면에서 svg 가 더 나은 거 같아요..!!
캡처한 화면으로는 잘 안보이는데 svg 가 벡터 이미지라 화면을 확대해도 화질은 더 좋아요 그리고 svg 로 만들면 나중에 sprite 로 한 svg 로 합쳐서 한번만 불러올 수도 있어서 최적화 할 수도 있을 거 같아용
| png | svg |
|---|---|
 |
 |
| png | svg |
|---|---|
 |
 |
eugene028
reviewed
Dec 30, 2024
| { | ||
| main: "L", | ||
| sub: "최소 규격은 가로 1280px, 그 이상의 모든 해상도에 적용되는 레이아웃으로, 데스크탑 환경에 대응해요.", | ||
| imageAlt: "breakpoint-4", |
There was a problem hiding this comment.
P3;
alt 이미지는 스크린 리더기에게 정보를 전달하는 것이기 때문에 이미지에 대해 구체적으로 묘사하도록 작성하는 것이 좋을 것 같아요~!
ex) 최소 규격 가로 1280px과 이상 모든 해상도에 대응하는 grid를 나타내는 이미지..
There was a problem hiding this comment.
요건 시간이 조금 걸릴 것 같아 다음 PR에서 수정할게요!
SeieunYoo
approved these changes
Jan 8, 2025
Comment on lines
+4
to
+8
| interface CardProps extends PropsWithChildren { | ||
| isBackground?: boolean; | ||
| style?: CSSProperties; | ||
| contentStyle?: CSSProperties; | ||
| } |
There was a problem hiding this comment.
p4;
className 도 받을 수 있도록 확장하면 좋을 거 같아요! clsx 를 활용하여 containerStyle 과 합치면 좋을 거 같습니당
There was a problem hiding this comment.
p3;
페이지에 쓰이는 데이터들은 const 폴더로 분리해보면 어떨까요?? 마크업과 분리되면 좋을 거 같아요
soulchicken
approved these changes
Jan 10, 2025
|
|
||
| export type Item = { | ||
| main: string; | ||
| sub?: string; |
There was a problem hiding this comment.
p3; sub의 타입을 저희가 쓰는 title 공통 컴포넌트에서처럼 ReactNode 타입을 사용하면 br 태그도 들어갈 수 있어서 좋을거같아요!
| Solid | ||
| </Text> | ||
| <Flex gap="1rem"> | ||
| {smallButtons.map((props) => ( |
There was a problem hiding this comment.
p4; Flex 태그의 자식 컴포넌트를 map을 통한 리스트를 만드셨기 때문에 각각 자식에게 key 값을 부여한다면 더 좋을거같아요.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎉 변경 사항
🙏 여기는 꼭 봐주세요!
아래 컴포넌트들은 공통적으로 사용할 수 있는 것들이라 눈여겨 봐주시면 좋을 것 같아요!
ComponentDetailTabsImageCardsCard컴포넌트에 스타일 및 백그라운드 조건부 추가