Skip to content
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

[공통] 헤더, 사이드 패널 컴포넌트화 #272

Merged
merged 6 commits into from
Apr 15, 2024

Conversation

junghaesung79
Copy link
Contributor

@junghaesung79 junghaesung79 commented Apr 15, 2024

What is this PR? 🔍

Changes 📝

  • 헤더, 사이드 패널에 대한 모든 내용이있던 컴포넌트의 가독성이 떨어져서 PCPanel과 MobilePanel 컴포넌트화 후 분리하였습니다.
  • 용어를 통일하였습니다.(megamenu, mega-menu => mega-menu)

ScreenShot 📷

Test CheckList ✅

Precaution

✔️ Please check if the PR fulfills these requirements

  • It's submitted to develop branch, not the main branch
  • Did you merge recent develop branch?

@junghaesung79 junghaesung79 added the 🔨 Refactor 코드 리팩토링 label Apr 15, 2024
@junghaesung79 junghaesung79 self-assigned this Apr 15, 2024
Copy link
Contributor

@dooohun dooohun left a comment

Choose a reason for hiding this comment

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

700줄 보고 기절할 뻔 했지만, 힘내서 리뷰했습니다 ㅎㅎ
수고하셨습니다

Comment on lines +12 to +13
LABEL1: 'mega-menu-label-1',
LABEL2: 'mega-menu-label-2',
Copy link
Contributor

Choose a reason for hiding this comment

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

변수명을 좀 더 구체적으로 표현하는게 좋을 거 같아요!

Copy link
Contributor

Choose a reason for hiding this comment

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

아 사용하지 않는 변수군요? 그렇다면 지워도 될 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

제 추측입니다만 추후 확장을 고려해서 예시 라벨을 넣어놓은 것 같습니다.

@@ -0,0 +1,264 @@
.mobileheader {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.mobileheader {
.mobile-header {

mobile-header 아닌가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

수정하였습니다!

@junghaesung79 junghaesung79 merged commit c436bcc into develop Apr 15, 2024
1 check passed
@junghaesung79 junghaesung79 deleted the feature/#271/header-componentize branch April 15, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 Refactor 코드 리팩토링
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[공통] 헤더, 사이드 패널 컴포넌트화
2 participants