-
Notifications
You must be signed in to change notification settings - Fork 23
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
[team#08 - FE] 1주차 PR #15
[team#08 - FE] 1주차 PR #15
Conversation
* #7 design: 디자인 시스템 세팅 (#8) * #7 design: 디자인 시스템 세팅 - 로고 컴포넌트, GlobalStyle, designSystem 생성 - icon svg 파일 업로드 - Pretendard font * #7 refactor: 로고 컴포넌트 props 구조 수정 * #7 refactor: 디자인 시스템 세팅 리뷰 반영 - design modules 불필요한 변수 선언 없이 export default - src 최상단 디렉터리 path alias 선언 * #7 design: light/dark 모드 design system 세팅 --------- Co-authored-by: 유지수 Jisoo Yoo <[email protected]>
* #10 design: Button component 구현 - Button type: container, outline, ghost - Button size: S, M, L - designSystem border color 제외 - GlobalStyle Button style reset 추가 * #10 design: TabButton 구현 & Button component 리팩토링 - Button 스타일 중복 삭제 BaseStyledButton - GhostButton 기반 TabButton 컴포넌트 구현 * #10 design: Button 내부 Icon 색상 변경 - filter 속성 추가 * #10 refactor: Button & TabBar 컴포넌트 리뷰 반영 - 컴포넌트 네이밍 수정 및 컨벤션 전반 적용 - TabBar 기본 상태 스타일 적용 * #10 refactor: 컨벤션 추가 적용
* #11 fix: lightMode.neutral.text.weak 값 수정 * #11 feat: Dropdown 관련 types 정의 * #11 feat: DropdownItem 구현 - DropdownItemType의 종류(`withImg`, `withColor`, `onlyContent`)에 따라 내부 구성 렌더. * #11 feat: DropdownPanel 구현 - `DropdownNameKOR` enum을 사용하여 영한 번역. - 로직은 영어로하고, 렌더할 때 한글로 번역. * #11 feat: DropdownIndicator 구현 - `isOpen`에 따라 `<DropdownPanel />` 렌더. - 사용 컴포넌트로부터 `dropdownName` 및 `dropdownList`를 props로 전달 받음. * #11 refactor: DropdownPanel의 종류화 - `DropdownPanel`이 사용되는 상황은 두가지(필터 또는 이슈 생성시 선택)가 있음. - 그에 맞게 `panelType`을 `filter`와 `select`로 구분하여 렌더. * #11 refactor: 프로퍼티명 변경 및 디자인 시스템 수정 - Dropdown 관련 types 정의에서 `type` -> `variant`으로 변경. - Design system에 `border` 및 SVG icon용 `filter` 추가. * #11 design: DropdownItem 선택시 폰트 적용 * #11 design: DropdownItem hover시 배경색 추가 * #11 design: DropdownIndicator를 버튼상태에 따라 opacity 적용 * #11 design: danger.surface.default 색상 추가 - 디자이너 추가사항 - 알럿창에 사용될 수 있음 * #11 feat: DropdownPanel "modify" 종류 추가 * #11 design: Design system border 및 filter 수정
- editor save 설정 - eslint-plugin-react 추가
- variant transient props로 수정
* #17 design: DropdownPanelType 수정 및 기타 UI 기능 추가 - 로직과는 별개로 단지 `DropdownIndicator`의 제목을 display할 때 쓸 `displayName`도 props로 받음. - `FilterBar`는 `DropdownIndicator`의 제목("필터")과 `dropdownName`("issue")이 다르기 때문. - `dropdownPanelPosition`을 props로 받아서 `DropdownPanel`을 좌/우로 붙일지 결정. - `onOutsideClick`을 통해 `DropdownPanel` 바깥을 클릭시 dropdown 닫힘. * #17 design: FilterBar Component UI * #17 refactor: DropdownPanel 인자에서 props destructuring * #17 rename: evt -> e
* #31 design: Logo 다크모드 적용 * #31 feat: ToggleSwitch component 구현 - Toggle이 필요한 곳에 재사용 가능 - 다크모드 토글할 때 사용할 예정 * #31 feat: Header component 구현 * #31 design: FilterBar input 폰트 적용 및 border-radius 수정 * #31 fix: TabBar에서 selectedTab 초기값을 null로 설정 * #31 design: InputCheckbox component 구현 - 커스터 checkbox input * #31 design: InputRadio component 구현 및 적용 - 커스텀 radio input * #31 design: MainPage 구조 설정 * #31 design: Table 관련 component 공통 UI 구현 - Table - TableHeader - 사용자가 내부 내용을 채움. - TableBody - 사용자가 내부 내용을 채움. * #31 design: TableHeaderIssues UI 구현 - `GhostButton`으로 처리한 부분 후에 `TabBar`로 변경 예정. - 추후에 props 내려받을 예정. * #31 design: IssuesPage, TableBodyIssues, IssueItem 초기 구현
- Button Type 별로 Styled-component 하위에 정의 - 불필요한 styled props $variant 전달 삭제
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.
조이, 카카모토비 반갑습니다 !! 😆
첫 주차인데 UI 마크업이 상당히 많이 진행되어서 깜짝 놀랐네요 👍
고생 많으셨습니당 ㅎㅎ
고민으로 남겨주신 내용은 이어서 댓글 남겨둘게요!
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.
eslint 설정 파일에 다양한 확장자를 사용할 수 있는데 한 번 훑어보심 좋을 것 같아 링크 남겨드려요!
https://eslint.org/docs/latest/use/configure/configuration-files
사용하신 eslint 버전이 8버전대라 위 문서 참고하시면 되고 9버전부터는 달라지는 부분이 있나보네용 👀
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.
기본 세팅 그대로 진행하면서 왜 cjs
확장자일지는 생각해보지 않았는데 짚어주셔서 감사합니다!
} | ||
}, | ||
"include": ["src"], | ||
"references": [{ "path": "./tsconfig.node.json" }] |
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.
references 옵션을 사용하신 이유가 궁금합니다!
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.
오, 초기화 시 불필요하게 남아있는 줄 알았는데 vite server에서 사용하는 타입 정보에 대한 것이었군요?! 놓칠 뻔 했는데 감사합니다.
// 0 <= opacity <= 1 | ||
export const decToHex = (opacity: number) => { | ||
return Math.floor(opacity * 255) | ||
.toString(16) | ||
.toUpperCase() | ||
.padStart(2, "0"); | ||
}; |
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.
opacity가 0과 1사이의 값이어야 한다면, 그 이외의 값이 들어왔을 때 에러를 던져주면 어떨까요??
}, | ||
"dependencies": { | ||
"axios": "^1.4.0", | ||
"msw": "^1.2.3", |
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.
msw는 개발 단계에서만 사용되기 때문에 devDependencies에 명시해주면 좋겠네요!
import { ReactElement } from "react"; | ||
import { styled } from "styled-components"; | ||
|
||
export default function TableBody({ children }: { children: ReactElement }) { |
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.
오호 저는 children 타입에 ReactNode를 쓰는데 문득 무슨 차이인지 궁금해서 이참에 찾아봤네요 ㅎㅎ
ReactElement, ReactNode, JSX.Element
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.
오 덕분에 React Element 관련 타입에 대해 정리했어요!
타입의 범위, 즉 포함 관계로 따지면 `ReactNode > ReactElement 이고, JSX.Element는 사실상 React에서는 React.Element와 동일한 타입이네요. 혹시 잘못된 부분이 있으면 알려주시면 감사하겠습니다 🙏
ReactNode
- 더 구체적으로 유형을 지정하지 않고 리액트에서 렌더링 가능한 모든 유형을 포괄하는 타입
- ReactElement,
ReactElement[]
, string, number, boolean 등
type ReactNode =
| ReactElement
| string
| number
| Iterable<ReactNode>
| ReactPortal
| boolean
| null
| undefined
| DO_NOT_USE_OR_YOU_WILL_BE_FIRED_EXPERIMENTAL_REACT_NODES[keyof DO_NOT_USE_OR_YOU_WILL_BE_FIRED_EXPERIMENTAL_REACT_NODES];
ReactElement
- ReactNode의 한 종류
- React.createElement가 반환하는 타입 (함수형 컴포넌트의 리턴값)
- props, state에 따라 컴포넌트가 렌더링할 UI 형태를 담은 객체
- 원시 타입을 허용하지 않고, 완성된 JSX Element만 허용
interface ReactElement<P = any, T extends string | JSXElementConstructor<any> = string | JSXElementConstructor<any>> {
type: T;
props: P;
key: Key | null;
}
JSX.Element
- JSX는 javascript 안에서 HTML 형태로 마크업을 작성할 수 있는 문법으로 실제 Typing은 다양하게 구현된다.
- 아래처럼 React 라이브러리에서 Global namespace에 ReactElement를 상속받은 인터페이스로서 JSX.Element type을 정의해둠
namespace JSX {
interface Element extends React.ReactElement<any, any> { }
참고
export default function Table({ children }: { children: ReactElement[] }) { | ||
return <StyledTable>{...children}</StyledTable>; | ||
} |
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.
children이 배열이어야 하나용? 👀
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.
아래 각 타입에 대해 정리해보니 children 타입은 ReactNode로 일관성있게 작성하는게 더 좋겠다는 생각이 드네요! @Kakamotobi 생각은 어떠신가요?
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.
안그러면 사용처에서 <></>
을 써야하는데 그러고 싶지 않아서 배열로 정의했습니다.
<Table>
<> // 안그러면 타입에러: single child을 기대하는데 multiple children을 제공함.
<TableHeaderIssues />
<TableBodyIssues issuesList={[{ title: "이슈 제목" }]} />
</>
</Table>
P.S. ReactNode는 복수(즉, ReactNode[])도 가능해서 아마 이 에러를 안마주치셨을 것 같습니다.
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.
오호 그러면 children을 다른 타입으로 정의해보는 건 어떨까요??
ReactElement, ReactNode, JSX.Element 요 타입들이 서로 슈퍼셋-서브셋 관계네요!
타입을 바꿔보면 타입을 배열로 하거나 <></>
를 쓰지 않고도 원하는 동작이 가능할 것 같아요
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.
ReactNode
로 바꿔도 무방하지만, ReactNode
는 text, number, boolean, 등 컴포넌트가 아닌 것 또한 포함하는데, 저희 구조상 Table
은 오직 컴포넌트(그것도 두개만)만 children
으로 받아야하기 때문에 제 생각에는 ReactElement
가 더 상황에 맞는것 같습니다. 👍
타입을 무언가의 배열로 표현하지 말아야할 어떤 이유가 있나요?
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.
타입에 배열을 사용하는 것 자체는 문제가 없는데요!
이 상황에서는 두 개 이상의 컴포넌트를 배열로 표현하는 것이 명시적이지는 않은 것 같아 말씀드려봤습니당 ㅎㅎ
추가로, Table 컴포넌트가 외부에서 children을 주입받기로 결정이 되었다면 children에 컴포넌트가 몇 개가 들어있든 그것은 Table의 관심사가 아니라는 생각이 드는데요.
children의 타입을 ReactElement[]
로 할 경우 children 컴포넌트를 하나만 넣어주는 상황에서는 에러가 발생하기 때문에 (= 즉 children에 Table이 관여하고 있음) 더 넓은 타입인 ReactNode를 써주는 게 좋지 않을까 하는 생각이 들었습니다!
import TableBodyIssues from "./TableBodyIssues"; | ||
import TableHeaderIssues from "./TableHeaderIssues"; | ||
|
||
export { Table, TableBodyIssues, TableHeaderIssues }; |
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.
index.ts에 모아서 export하는 방식을 선택하신 이유가 궁금해요!
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.
/Table
폴더안에 내부적, 외부적으로 쓰일 모듈들이 있는데 외부에서 사용될 것만 모아서 export하고 싶었습니다.
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.
하나의 컴포넌트를 구성하는 하위 컴포넌트의 크기가 커서 여러 모듈로 분리할 때는 외부에서 실제로 사용하게 되는 컴포넌트를 분명하게 하기 위해 index.ts에서 대표 컴포넌트를 export하는 방식을 선택하고 있어요! 저는 비슷한 맥락으로 이해했습니다.
export const postSignup = async (loginId: string, password: string) => { | ||
await instance.post("/auth/signup", { loginId, password }); | ||
}; |
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.
이렇게 axios를 한번 랩핑해서 사용하는 방식 저는 좋다고 생각해요~~
만약에 라이브러리를 바꿔야 하는 상황이 오면, 사용처를 다 돌아다니지 않고 이 파일에서만 수정하면 되니까요!
조금 더 고도화를 해보면 좋을 것 같은데 여기서 곧바로 axios를 사용하는 것이 아니라
계층을 하나 더 두어서 (이름은 보통 Fetcher라고 부르는 것 같아요?) fetcher.post(~~~)
라고 작성해준다면
라이브러리를 바꿔야 하는 상황이 왔을 때 Fetcher 내부만 수정하면 되기 때문에 훨씬 더 변경에 유연합니다!
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.
오 fetcher 추상화 계층을 중간에 두면 경우에 따라 인터페이스가 동일한 다른 모듈로 손쉽게 변경해줄 수 있겠군요. 만약의 상황이지만 axios 라이브러리 내부 문제 발생 시 다른 라이브러리로 구현된 fetch 기능을 활용할 수 있도록!?
return { | ||
value, | ||
onChange, | ||
minLength, | ||
maxLength, | ||
isValid: isValid.current, | ||
}; |
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.
useInput은 validation을 위한 목적으로 만들어진 것 같군요?
조금 더 확장성을 고려해보면 length 말고도 다양한 validation이 있을 수 있는데 (이메일형식 체크, 숫자만 입력했는지 체크 등등)
이런 모든 validation 처리가 가능하려면 어떻게 인터페이스를 구성하면 좋을지 고민해보셔도 좋을 것 같아요 👍
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.
네 이번 요구사항에는 최소, 최대 길이밖에 없지만 실제 서비스에서는 당연히 훨씬 복잡한 validation이 필요할 것이고 로직을 분리하면 좋을 것 같았어요!
추후에 인터페이스에 validations을 추가하고 각 validation 함수를 util에 별도로 분리하여 관리하고, useInput의 인자로 넘겨서 처리해주는 방식으로 생각해두었어요.
type ValidationFunction = (value: string) => boolean;
type useInputOptions = {
initialValue: string;
minLength?: number;
maxLength?: number;
validations?: ValidationFunction[];
};
재사용성을 고려한 컴포넌트의 크기저도 이게 항상 고민입니다. ㅋㅋㅋㅋ 새로운 라이브러리 학습 & 적용 TIP!?새로운 라이브러리는 아무래도 낯설기 때문에 처음엔 시간이 좀 걸리기는 하죵 ㅠㅠ 라이브러리를 학습하는 게 목적이라면 저는 어떤 인터페이스로 구성되어 있는지(= 어떤 요소를 외부에 노출하는지, 예를 들면 props 같은), 내부 로직은 어떻게 구현되어 있는지, 프로젝트 구조는 어떻게 되어있는지 이런 것들을 직접 코드 까보면서 구경해보고 있어요. 😄 선택의 갈래??아마 기술적인 부분에서의 선택을 말씀하신 거겠죠?? 라이브러리를 선택해야 하는 상황이라면
로직, 리팩토링이라면 사람마다 너무 기준이 달라서 더 어려운데,, 저는 보통
|
- availableBold -> availableMedium
- 스타일드 컴포넌트 확장, 오버라이딩으로 스타일 코드 복잡도 줄임
깃허브에서 name 을 설정하지 않는 사용자면 null 값이 담기게 되어서 변경
- 토큰 insert or update 부분 조건문에서 메서드로 분리
- 깃허브 이메일이 public 상태가 아니면 https://api.github.com/user 에서 email 정보를 받아오지 못하여서 email을 저장하지 못하고 email 을 사용하는 부분에서 오류 발생 - https://api.github.com/user/emails 로 따로 요청을 하면 public 상태가 아니어도 사용자의 이메일을 가져올 수 있음
- 자체 회원가입 한 유저의 이메일이 깃허브 로그인 이메일과 같을 때 해당 유저의 정보를 깃허브 정보로 갱신 하려 했으나 로그인을 하면서 회원정보를 갱신하는 것은 로그인이 너무 많은 일을 하게 되는 것 같아서 깃허브로 로그인/회원가입 한 유저는 자체 로그인을 이용하지 못하게 변경
[BE] ERD를 참고하여 테이블 구현 및 예외처리 틀 구현
Issue Tracker 1주차
안녕하세요 Autumn! 이번 Issue Tracker 프로젝트 리뷰를 받게된 카카모토비, 조이입니다. 앞으로 잘부탁드려요 😆
이번 첫주차에는 진행 방식에 대해 논의하고, 이에 맞춰서 작업을 해나가는 단계랍니다.
우선, 현재 진행 상황은 아래와 같이 실행하셔서 확인하실 수 있어요!
(단, 아직 route 이동 기능 구현 전이라 직접 path를 입력해야 페이지 확인이 가능하답니다 ..😉)
실행 방법
git clone https://github.com/issue-tracker-08/issue-tracker-max.git git switch fe-w1 cd fe npm i npm run dev
협업 방식
프론트 협업 방식
개발 환경 설정
컴포넌트 구조 설계
디자인 시스템
공통 컴포넌트 작업
Router 초기 세팅
AuthPage, MainPage
고민 & 질문
재사용성을 고려한 컴포넌트의 크기
새로운 라이브러리 학습 & 적용 TIP!?