-
Notifications
You must be signed in to change notification settings - Fork 8
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
템플릿 공개 범위 설정(visibility) 기능 구현 #787
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
빠른 시간안에 구현해주셨네요 헤인!
점점 실력이 느는게 보여서 대단합니다.
코드 관련 사항은 코멘트 남겼고, 디자인 관련해서는 내일 상의해보아요.
예를 들어, visibility 토글을 저장 버튼 옆에 둘건지 등등,,
고생하셨습니다~!
const [showAllTagList, toggleShowAllTagList] = useToggle(); | ||
const isPrivate = visibility === 'PRIVATE'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 문자열도 constant로 관리하면 좋겠네요!
left: 2px; | ||
transform: ${({ isRight }) => (isRight ? 'translateX(calc(100% - 4px))' : 'translateX(0)')}; | ||
|
||
width: calc(50%); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요건 어떤 식인가요?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오른쪽 옵션을 선택하면 오른쪽으로 슬라이더를 보내주는 부분입니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
width: calc(50%);
부분이면 그냥 50%랑 같지 않을까 해서요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앗, 그 부분은 50%랑 동일합니다! (원래 계산식이 있었는데 수정하면서 빠졌나봐요)
width: 50%;
으로 수정해두겠습니다!! 👍
padding: 0.5rem 0 0 0; | ||
padding-top: 0.5rem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍👍
optionAdornments = [undefined, undefined], | ||
optionSliderColor = [undefined, undefined], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 값들을 undefined로 기본값을 줘야하는 이유가 뭔가요?! 혹시 이후 사용처에서 옵셔널 체이닝을 쓰지 않기 위함인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 아래와 같이 구조분해 할당을 해서 사용하고 있는데,
const [leftOptionAdornment, rightOptionAdornment] = optionAdornments;
해당 값들에 기본값을 주지 않으면 아래와 같은 에러가 납니다.
'[ReactNode, ReactNode] | undefined' 형식에는 반복기를 반환하는 '[Symbol.iterator]()' 메서드가 있어야 합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
두 props를 분리해서 쓰면 더 명확해지는 대신 props가 많아질까요?
<PrivateIcon key={TEMPLATE_VISIBILITY[1]} width={18} />, | ||
<PublicIcon key={TEMPLATE_VISIBILITY[0]} width={18} />, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
key가 필요한 이유가 뭔가요?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아래와 같은 eslint 에러를 해결하기 위해 key 를 넣어주었습니다.
Missing "key" prop for element in array
개인적인 추측으로는 인자로 전달되는 ReactNode 이기 때문에 key 값으로 변경을 감지하기 위해 경고해주는 것 아닐까 싶습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오홍 아마 두 아이콘이 배열로 전달되어서 그런 것 같네용
<Flex gap='0.5rem'> | ||
{isPrivate && ( | ||
<S.PrivateWrapper> | ||
<PrivateIcon width={14} color={theme.color.light.black} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
원래 컬러가 black 아닌가요? 다른 사용처에서는 color 속성을 주지 않길리 질문드림다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 PrivateIcon
과 PublicIcon
은 상위 컨텐츠의 색상을 따르도록 되어있으나(fill="currentColor"
), 해당 부분에서 왜인지 #393e46
값을 가지더라고요. 제목과 같은 줄에 있으므로 제목과 같은 컬러를 사용해주는 것이 좋을 것 같아 위와 같이 색상을 지정해주었습니다.
@@ -34,6 +35,7 @@ const TemplateCard = ({ template }: Props) => { | |||
<Flex width='100%' direction='column' gap='1rem'> | |||
<Flex width='100%' justify='space-between' gap='1rem'> | |||
<Flex gap='0.75rem' flex='1' style={{ minWidth: '0' }}> | |||
{isPrivate && <PrivateIcon width={14} color={theme.color.light.secondary_800} />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아이콘의 width를 14나 18 처럼 쓰고있는데, 이런 매직넘버를 줄이고 나중에는 여기서는 몇을 써야하지? 같은 상황을 방지하기 위해 해당 icon을 svg component롤 만들어서 해당 값들을 의미 있는 변수로 할당하면 좋겠네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 다른 PR에서도 느꼈어요! 스타일 관련된 상수 파일을 하나 만들어서 정리해볼게요!
컴포넌트를 만들지 않고, 아이콘 사이즈에 대한 상수만 선언해서 사용한 이유는, 현재 저희 프로젝트 내부에서 다양한 아이콘을 사용하고 있는데, 그 모든 것을 매번 컴포넌트로 만들기보다는 정해진 사이즈 내에서 적절한 사이즈를 선택하여 사용하는 것이 더 편할 것 같다고 생각했기 때문입니다.
2024-10-17.6.02.04.mov최종 디자인 결과입니다! |
width={ICON_SIZE.LARGE} | ||
height={ICON_SIZE.LARGE} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 이렇게 처리를 했군요?! 사실 상수로 할 생각은 안해봐서 신박했습니다.
원래는 HamburgerIcon
에 Props를 필요에 따라 medium, large 이런 식으로 하려 했거든요.
다만 상수로 하니까 애매한 건 MEDIUM_LARGE 같은 상수가 나와버리네요..! ㅋㅋㅋ
…de-zap into feat/739-private
* 문의하기 모달 생성 (#777) * design(Header): border-bottom 1px 로 변경 및 색상 secondary_300으로 변경 * feat(src): Textarea 컴포넌트 생성 * refactor(src): getChildOfType, getChildrenWithoutTypes 함수 utils로 파일로 분리 * feat(src): ContactUs 컴포넌트 생성 * feat(Modal): Modal 컴포넌트에 usePressESC, useScrollDisable 적용 * feat(components): Header 및 Footer에 ContactUs 적용, Header 에서 현재 경로 스타일 변경 * feat(ContactUs): 구글 시트 연동 추가 * refactor(ContactUs): isValidContents 상태 boolean 으로 관리하도록 변경 * design(Header): 헤더 '코드잽' secondary_800 으로 색상 변경 * refactor(Header): 헤더 '코드잽' 로고에도 현재 경로 스타일 적용 * 태그 버튼 색상 적용 (#765) * feat(style): 태그 버튼 팔레트 정의 * feat(utils): 태그 버튼의 색상 선택 유틸함수 구현 * feat(src): 태그 버튼, 필터 디자인 변경 및 태그 색상 적용 * refactor(utils): getTagColor 유틸함수를 컴포넌트 내부에 구현 * refactor(TagInput): 템플릿 업로드/수정 중 태그 색상 기본색으로 변경 * refactor(src): 태그 필터에서 선택된 태그 구분을 위한 디자인 수정 * fix(TemplateCard): SourceCodeViewer에 filename 전달 * refactor(TagButton): 함수 분리 및 메모이제이션 수행 * refactor(TagButton): 불필요한 useMemo 제거 --------- Co-authored-by: 헤인 <[email protected]> * api 변경에 따른 /login 엔드포인트 제거 (#808) * 카테고리, 태그, 템플릿 목록 로딩 처리 개선 (#813) * refactor(components): ScrollTopButton 컴포넌트 분리 * refactor(MyTemplatePage): 컴포넌트 분리 및 태그목록, 카테고리 목록 서스펜스 적용 * refactor(components): TemplateDeleteSelection, TemplateListSectionLoading 컴포넌트 분리 * refactor(src): useKeyword, useShowTemplateList, useSelectAndDeleteTemplateList 훅 분리 및 MyTemplatePage 적용 * refactor(templates): 사용하지 않는 useTemplateCategoryTagQueries 훅 삭제 * refactor(pages): useShowTemplateList => useFilteredTemplateList로 이름 변경 * refactor(src): useKeyword => useSearchKeyword로 이름 변경 * refactor(ConfirmDeleteModal): ConfirmDeleteModalProps =>Props 로 변경 * refactor(CategoryListSection): Flex 대신 스타일드 컴포넌트로 스타일 지정 * refactor(ConfirmDeleteModal): Modal 합성 컴포넌트의 하위 컴포넌트 사용하도록 변경 * refactor(queries): useSuspenseQuery를 사용하는 훅들 수동으로 error 전파 * 템플릿 공개 범위 설정(�visibility) 기능 구현 (#787) * feat(components): Toggle 컴포넌트 * feat(images): PrivateIcon, PublicIcon 추가 * refactor(mocks): templateList mock data에 "visibility" 추가 * feat(src): visibility 기능 추가 * feat(src): visibility가 private인 경우 템플릿 카드, 상세 페이지에서 privateIcon 보이도록 설정 * refactor(TemplateCard): 사용하지 않는 스타일드 컴포넌트 제거 * refactor(src): PRIVATE 상수화 * refactor(Toggle): Toggle.style 에서 불필요한 calc() 제거 * refactor(src): ICON_SIZE 상수 선언 및 적용 * refactor(components): CategoryDropdown 스타일 변경 * refactor(Toggle): Toggle 스타일 변경 및 showOptions 생성 * refactor(pages): 템플릿 생성 및 수정 페이지 Toggle 변경사항 반영 * ApiError 및 ApiClient 생성으로 HTTP 요청 및 에러 핸들링 리팩토링 (#783) * feat(Error): ApiError 객체 생성 * feat(api): ApiClient 객체 생성 및 template 구경가기, 업로드 요청 적용 * chore: react-error-boundary 라이브러리 설치 * feat(TemplateExplorePage): 일시적 에러에 대한 Local ErrorBoundary 적용 * feat(api): ApiClient - 인증 실패 시, localStorage에서 유저 정보 제거 * refactor(api): templates - JSON.stringify 중복 적용 제거 * feat(api): ApiClient - 응답 본문이 없을 때, 반환값 처리 * refactor(api): getLoginState - apiClient 적용 및 중복 코드 제거 * refactor(api): ApiClient - fetch문에 try-catch 추가, error handling 메서드 분리 * feat(frontend): setupTests - MSW 서버의 unhandled handler 디버깅 코드 추가 * refactor(api): API_URL에 기본값 문자열 추가 * refactor(api): ApiClient - 에러 확인 분기문을 hasError 메서드 밖으로 분리 * refactor(Error): TempError -> TemporaryError로 컴포넌트 명 변경 * refactor(api): ApiClient - method의 타입을 string에서 HttpMethod 타입 정의 * refactor(Error): ApiError - switch-case 문을 매핑으로 변경 * refactor(Error): errorCode, detail에 기본값 정의 * refactor(api): errorCode의 타입을 string -> number로 변경 * refactor(Error): TemporaryError - 불필요한 조건문 제거 * refactor(api): ApiClient - customFetch에서 response body가 아닌 response 자체를 반환하도록 변경 * refactor(templates): useTemplateUploadMutation - 요청 성공에 따라 location 라우트 이동, 에러 처리 변경 * refactor(api): templates - Endpoint 상수화 * refactor(templates): useTemplateUploadMutation - wrapper 추가 * feat(src): 확장자 기반으로 언어 태그 자동 생성 기능 추가 (#814) * chore: version 1.1.4로 변경 (#817) --------- Co-authored-by: 월하 <[email protected]> Co-authored-by: MYONG JAEWI <[email protected]>
⚡️ 관련 이슈
🪧 구현 사항
visibility 선택 UI는 아래와 같이 토글로 구현하였습니다.
2024-10-15.2.38.15.mov
템플릿 카드에서 private 표시
상세페이지에서 private 표시
📍주요 변경 사항
1. Toggle 컴포넌트 생성
2. Template 관련 타입에 visibility 추가
constants 파일
에서TEMPLATE_VISIBILITY
를 선언하고, 이를 바탕으로 타입TemplateVisibility
을 추출하였습니다.3.
TemplateUploadPage
,TemplateEditPage
에 visibility Toggle 추가4. visibility가 private인 경우 템플릿 카드, 상세 페이지에서 privateIcon 보이도록 설정
🎸기타
TemplateUploadPage
,TemplateEditPage
두 파일 모두 수정해주어야 해서 굉장히 번거롭습니다...🥲 이 부분에 대해 또 다시 추상화를 진행해도 괜찮을까요?Toggle
컴포넌트가 생기면서useToggle
이 마치 이 컴포넌트를 위해 존재하는 훅처럼 느껴질까봐 조금 걱정이 됩니다. 현재Toggle
컴포넌트는useToggle
을 사용하지 않습니다. option을 T 로 받고, selectedOption 또한 T 로 관리하기 때문입니다. 커스텀 훅의 이름을 위와 같이 지은 이유는 boolean 값을 토글처럼 바꾼다는 의미에서 가져온 것인데 자칫 혼동을 줄 수 있을 것 같아 한 번 더 언급해봅니다. 혹시 계속 혼동될 것 같다면 커스텀 훅 네이밍을 바꿔도 좋을 것 같습니다.🍗 PR 첫 리뷰 마감 기한
10/16(수) 18:00