Skip to content

Conversation

@lalaurrel
Copy link
Collaborator

주요 작업 내용

  • profile, profileEdit, accountEdit, accountCancel, accountSetting, verification 기존 폰트 및 색상 삭제 후 수정했습니다
  • 해당 페이지들에서 절대경로로 수정했습니다
  • 충돌해결했습니다!

기타 작업 내용

  • 없음

코드 리뷰 포인트

  • 없음

작업 화면

  • 없음

@lalaurrel lalaurrel added fix Fix bugs that don't work refactor Improve code structure and readability labels Dec 28, 2024
@lalaurrel lalaurrel requested a review from gustn99 December 28, 2024 15:40
@lalaurrel lalaurrel self-assigned this Dec 28, 2024
Copy link
Collaborator

@gustn99 gustn99 left a comment

Choose a reason for hiding this comment

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

아래 요소들에 대해 전반적인 점검이 필요합니다!

  1. import문 점검
    a. 상대 경로에서 index 파일을 import할 때에는 '.../index'와 같이 /index 필요
    b. 모든 경로에서 dto 파일을 import할 때에는 import type {...} from '.../dto'처럼 type 필요
  2. 컬러시스템 점검
    a. 컬러시스템이 text/background/border에 대해 세분화되어 있어, 이를 활용하는 방향으로 적용해 주시면 좋을 것 같습니다
  3. 예전 폰트 스타일 삭제
    a. 작업하신 영역에서 아직 예전 폰트 스타일이 일부 남아 있습니다! 코멘트 남겨두었으니 확인하셔서 수정 부탁드립니다
  4. 인라인 css 삭제
  5. 이 외에는 코멘트 참고해 주시면 되겠습니다!

이 브랜치가 머지된 후에는 새 브랜치에서 아래 작업이 추가로 필요합니다

  1. 변수 명명 규칙에 따라 변수명 점검
  2. 전체 코드 흐름 점검
  3. ConfirmationModal -> Modal로 대체

아자아자 파이팅입니다다다다!!!

<label style={{ display: 'flex', alignItems: 'center', cursor: 'pointer' }}>
<CheckboxInput type="checkbox" checked={isChecked} onChange={handleCheckboxChange} />
<StyledText as="span" $textTheme={{ style: 'body4-light', lineHeight: 1 }} color={theme.colors.gray3}>
<StyledText as="span" $textTheme={{ style: 'body4-light', lineHeight: 1 }} color={theme.colors.gray[600]}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

StyledText 등 폰트에 대해서는 되도록 theme.colors.text에 있는 값을 사용해 주시면 좋을 것 같아용 추가로 lineHeight 삭제해 주세요! 이 외에 다른 StyledText나 스타일드 컴포넌트 내 color 속성도 한번씩 점검 부탁드립니다!!

Copy link
Collaborator

Choose a reason for hiding this comment

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

또한 여기에 사용된 body4-light는 예전 폰트 스타일입니다! 수정 부탁드려용

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

theme.colors.text로 다 수정했습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

lineHeight 다 삭제했습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

예전 폰트 다 수정했습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아 아직 수정중이야!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

내가 보려고.. 쓰는중 ..

Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋ 조아요 파이팅.....

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

theme.colors.background에 없는 컬러들을 쓰고 있는 background는 일단 그냥 둘게요 .. ㅜ

Copy link
Collaborator

Choose a reason for hiding this comment

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

넵!! background나 border에 없는 색들은 최대한 colors 내부에서 뽑아쓰고 그마저도 안 되면 일단은 색상코드나 rgb 쓰는 식으로 해주세용

</InfoItem>
</InfoBox>
<CheckboxWrapper as="div">
<label style={{ display: 'flex', alignItems: 'center', cursor: 'pointer' }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

인라인 css는 더 이상 사용하지 않기로 결정했습니다! 대신에 class나 styled component 사용해 수정해 주세용

export const InfoBox = styled.div`
background: #f5f5f5;
padding: 70px; /* 20px */
background: ${({ theme }) => theme.colors.gray[100]};
Copy link
Collaborator

Choose a reason for hiding this comment

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

background에 사용되는 색상은 theme.colors.background에 정리되어 있습니다! 이걸 사용해 주시면 유지보수 측면에서 더 좋을 것 같아요 여기 말고도 다른 background도 한번씩 점검해 주세용

width: 1.25rem;
height: 1.25rem;
border: 0.125rem solid #e0e0e0;
border: 0.125rem solid ${({ theme }) => theme.colors.gray[200]};
Copy link
Collaborator

Choose a reason for hiding this comment

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

theme.colors.border 속성을 사용해 주시면 좋을 것 같아요!

Comment on lines 88 to 103
export const StyledButton = styled.button<ButtonProps>`
margin-top: 18.75rem; /* 300px */
background: ${(props) => (props.isChecked ? 'black' : '#ccc')};
border-radius: 0.5rem; /* 8px */
margin-top: 18.75rem;
background: ${({ theme, isChecked }) =>
isChecked ? theme.colors.black : theme.colors.gray[300]};
border-radius: 0.5rem;
border: none;
padding: 1.5625rem; /* 25px */
padding: 1.5625rem;
text-align: center;
font-size: 1rem; /* 16px */
color: white;
font-size: 1rem;
color: ${({ theme }) => theme.colors.white};
cursor: ${(props) => (props.isChecked ? 'pointer' : 'not-allowed')};
&:disabled {
background: #00000080;
background: ${({ theme }) => `${theme.colors.black}80`};
}
`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

사용되지 않는 컴포넌트 같습니다! 확인 부탁드려요

import Loading from '@components/Loading';
import camera from '@assets/default/camera.svg';
import { getUserInfoApi, patchUserInfoApi } from '@apis/user';
import { UserInfoData, PatchUserInfoRequest } from '@apis/user/dto';
Copy link
Collaborator

Choose a reason for hiding this comment

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

dto를 import할 때는 import type {...} from '.../dto'와 같이 작성해 주셔야 린트가 적용될 수 있으니 수정 부탁드립니다!

import { StyledText } from '@components/Text/StyledText';
import theme from '@styles/theme';

const ButtonSecondary: React.FC = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

요기 button1-regular는 예전 폰트 스타일입니다! 수정 부탁드립니닷 color도 수정이 필요할 것 같네용

height: 3.1rem;
text-align: center;
color: #ff2389;
color: ${({ theme }) => theme.colors.brand.primary};
Copy link
Collaborator

Choose a reason for hiding this comment

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

color 속성을 버튼에 부여하는 대신 StyledText에서 theme.colors.brand.primary로 설정하면 버튼에서는 color 속성을 삭제해도 될 것 같아 보이네요!

Comment on lines 31 to 32
import { UserPostSummary } from '@apis/post/dto';
import { UserInfoData } from '@apis/user/dto';
Copy link
Collaborator

Choose a reason for hiding this comment

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

dto를 import할 때는 import type으로 작성해 주셔야 린트 설정으로 import문 정렬이 가능해지니 수정 부탁드립니다! 다른 부분도 한번씩 점검 부탁드려용

Comment on lines 15 to 16
import NavbarProfile from './NavbarProfile';
import NavBar from '../../components/NavBar';
import ButtonSecondary from './ButtonSecondary';
Copy link
Collaborator

Choose a reason for hiding this comment

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

상대경로로 index 파일을 import할 때에는 경로에 /index를 작성해 주셔야 린트가 올바르게 적용될 수 있습니다! 수정 부탁드려요

@lalaurrel lalaurrel requested a review from gustn99 December 28, 2024 19:55
@gustn99 gustn99 merged commit 2083817 into dev Dec 29, 2024
1 check passed
@gustn99 gustn99 deleted the feat/OD-150 branch December 29, 2024 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Fix bugs that don't work refactor Improve code structure and readability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants