Skip to content

Commit

Permalink
Merge pull request #46 from kakao-tech-campus-2nd-step3/Master
Browse files Browse the repository at this point in the history
6주차 코드리뷰 PR
  • Loading branch information
JunilHwang authored Oct 13, 2024
2 parents 6e952be + 3c9811c commit adbfaac
Show file tree
Hide file tree
Showing 83 changed files with 2,392 additions and 525 deletions.
16 changes: 6 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,18 @@
# 🍪 내가 먹은 쿠키 - 18조 FE

## 🙋‍♂️ 4주차 코드리뷰 질문

- `Modal` 컴포넌트를 구현하면서 텍스트 부분과 버튼 부분에 들어갈 내용은 개발할 때 코드를 작성하는 사람이 자유롭게 작성하여 구성할 수 있도록 `textChildren``buttonChildren`만으로 구성하였는데, 더 적합하거나 지향하는 방식이 있을까요?
## 🙋‍♂️ 6주차 코드리뷰 질문
- 하나의 파일 내에서 함수 표현식과 선언식을 같이 사용해도 되는지 궁금합니다.
- 기존 Weekly 브랜치에 components 경로에 `RoleModal` 컴포넌트가 있다고 가정하면, 해당 Weekly브랜치를 통해 새로 분기된 Feat 브랜치에 기존 components에 `RoleModal` 컴포넌트를 features 경로에 옮기고 두 브랜치를 머지하게되면 두개의 파일이 생기게 됩니다. 이를 해결하려면 어떻게 해야하나요?

## 🙋‍♂️ 5주차 코드리뷰 질문

- 하나의 페이지 내에서만 여러번 사용되는 공통 컴포넌트의 경우, components/common 폴더에 공통 컴포넌트로 만들어 취급하는 것이 좋은지, 혹은 해당 페이지 코드 파일이 위치한 폴더에 컴포넌트를 만들거나 해당 페이지 코드 파일 하단에 작성하는 등 colocation 원칙을 적용해서 가까이 위치시키는 것이 좋을지 궁금합니다.
- `Header` 컴포넌트에서 다른 theme을 가진 버튼들에 공통된 스타일을 적용하면서, 특정 버튼에만 추가적인 스타일을 주는 작업을 했습니다. 아래와 같이 각 버튼에 공통적으로 적용될 스타일을 `commonButtonStyles`로 정의하고, `theme=default`인 버튼에만 추가 스타일을 적용해보았는데, 제가 구현한 방식보다 더 괜찮은 방법이 있는지 궁금합니다.

```jsx
const commonButtonStyles = css`
white-space: nowrap;
border-radius: 4px;
`;

```

```
<Button theme="outlined" css={[commonButtonStyles]}>
채용공고 등록
Expand All @@ -35,12 +31,12 @@ const commonButtonStyles = css`
>
로그아웃
</Button>
```

- 태블릿(`768px`)과 모바일(`480px`)에서 반응형을 고려하여 `breakpoints`를 정의하였고, 이를 보다 명시적으로 활용하기 위해 `responsiveStyles` 함수를 구현했습니다.
멘토님께서는 보통 반응형 스타일링을 구현할 때 어떤 방식을 사용하시나요?
혹시 제가 사용한 `responsiveStyles` 함수보다 효율적이거나 코드의 가독성을 높일 수 있는 더 나은 방법이 있을까요? 멘토님이 추천하는 방법이나 일반적으로 사용되는 best practice 또한 궁금합니다.

- 현재 `Modal` 컴포넌트를 사용할 때마다 `useToggle` 커스텀 훅을 함께 사용해야 해서, 모달을 제어하기 위한 코드가 흩어져 있는 느낌입니다. 이렇게 되면 모달 관련 로직으로 인해서 단일 책임 원칙에 어긋난다는 생각이 들곤 합니다.
보다 나은 방식으로 `Modal` 컴포넌트를 동작시킬 수 있는 방법이 있을까요? `useToggle`처럼 모달을 제어하는 로직을 간소화하고, 모달 컴포넌트 자체가 스스로 상태를 관리하거나 쉽게 제어 가능한 형태로 구현할 수 있는지 궁금합니다.

## 🙋‍♂️ 4주차 코드리뷰 질문
- `Modal` 컴포넌트를 구현하면서 텍스트 부분과 버튼 부분에 들어갈 내용은 개발할 때 코드를 작성하는 사람이 자유롭게 작성하여 구성할 수 있도록 `textChildren``buttonChildren`만으로 구성하였는데, 더 적합하거나 지향하는 방식이 있을까요
1 change: 1 addition & 0 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export default tseslint.config(
rules: {
...reactHooks.configs.recommended.rules,
'react-refresh/only-export-components': ['warn', { allowConstantExport: true }],
'react-refresh/only-export-components': 'off',
},
},
);
104 changes: 102 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"csstype": "^3.1.3",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-hook-form": "^7.53.0",
"react-router-dom": "^6.26.2",
"zustand": "^4.5.5"
},
Expand All @@ -47,6 +48,7 @@
"@storybook/test": "^8.3.0",
"@types/react": "^18.3.3",
"@types/react-dom": "^18.3.0",
"@types/react-slick": "^0.23.13",
"@typescript-eslint/eslint-plugin": "^8.5.0",
"@typescript-eslint/parser": "^8.5.0",
"@vitejs/plugin-react": "^4.3.1",
Expand All @@ -60,6 +62,8 @@
"lint-staged": "^15.2.10",
"msw": "^2.4.6",
"prettier": "3.3.3",
"react-slick": "^0.30.2",
"slick-carousel": "^1.8.1",
"storybook": "^8.3.0",
"typescript": "5.5",
"typescript-eslint": "^8.0.1",
Expand Down
Empty file.
1 change: 1 addition & 0 deletions src/apis/auth/mock/index.mock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// 데이터 관련된
4 changes: 4 additions & 0 deletions src/assets/icons/arrow/right-blue.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions src/assets/icons/arrow/right-white.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 11 additions & 0 deletions src/assets/icons/companyInfo/brand.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions src/assets/icons/companyInfo/industry.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions src/assets/icons/companyInfo/revenue.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions src/assets/icons/navigation/menu-close.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions src/assets/icons/navigation/menu-open.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions src/assets/styles/theme.ts → src/assets/theme.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Theme } from '@emotion/react';
import { palettes } from './global/palettes';
import { mediaQueries } from './global/breakpoints';
import { palettes } from './styles/global/palettes';
import { mediaQueries } from './styles/global/breakpoints';

const theme: Theme = {
mediaQueries,
Expand Down
12 changes: 11 additions & 1 deletion src/components/common/Flex/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ type FlexGap = {
interface FlexProps {
direction?: 'column' | 'row';
gap?: FlexGap;
wrap?: boolean;
justifyContent?: CSS.Properties['justifyContent'];
alignItems?: CSS.Properties['alignItems'];
}
Expand All @@ -19,13 +20,21 @@ type Props = HTMLAttributes<HTMLDivElement> & FlexProps;
export default function Flex({
direction = 'row',
gap,
wrap = false,
justifyContent = 'start',
alignItems = 'start',
children,
...rest
}: Props) {
return (
<Container direction={direction} gap={gap} justifyContent={justifyContent} alignItems={alignItems} {...rest}>
<Container
direction={direction}
gap={gap}
justifyContent={justifyContent}
alignItems={alignItems}
wrap={wrap}
{...rest}
>
{children}
</Container>
);
Expand All @@ -34,6 +43,7 @@ export default function Flex({
const Container = styled.div<FlexProps>`
width: 100%;
display: flex;
flex-wrap: ${({ wrap }) => (wrap ? 'wrap' : 'nowrap')};
flex-direction: ${(p) => p.direction};
justify-content: ${(p) => p.justifyContent};
align-items: ${(p) => p.alignItems};
Expand Down
9 changes: 9 additions & 0 deletions src/components/common/Icon/Arrow.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import RightWhite from '@assets/icons/arrow/right-white.svg?react';
import RightBlue from '@assets/icons/arrow/right-blue.svg?react';

const Arrow = {
RightWhite,
RightBlue,
};

export default Arrow;
Loading

0 comments on commit adbfaac

Please sign in to comment.