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

[버스] 시간표 페이지 개편 #613

Merged
merged 20 commits into from
Dec 31, 2024
Merged

Conversation

JeongWon-CHO
Copy link
Contributor

What is this PR? 🔍

Changes 📝

버스 시간표 페이지(bus-course)를 개편했습니다.

ScreenShot 📷

  • 메인화면
image
  • 상세 시간표 1
image
  • 상세 시간표 2
image
  • 대성고속 시간표
image
  • 시내버스 시간표
image

Test CheckList ✅

  • 공지사항 클릭 시, 해당 공지사항으로 이동되는지

Precaution

✔️ Please check if the PR fulfills these requirements

  • It's submitted to the correct branch, not the develop branch unconditionally?
  • If on a hotfix branch, ensure it targets main?
  • There are no warning message when you run yarn lint

@JeongWon-CHO JeongWon-CHO self-assigned this Dec 25, 2024
Copy link
Contributor

@junghaesung79 junghaesung79 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 급한 pr인 것 같아 일단 어프루브 드립니다. 하지만 바로 머지하지 마시고 리뷰 내용 읽어보면서 적용할 부분 적용하시면 좋을 것 같아요!

Comment on lines 50 to 51
.sub-title {
color: #000;
Copy link
Contributor

Choose a reason for hiding this comment

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

subtitle이 한 단어이기 때문에 하이픈으로 나누지 않아도 좋을 것 같아요!

Comment on lines 48 to 52
@each $type, $color in $template-shuttle-colors {
.type-#{$type} {
background-color: $color;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

오! 이런 식으로 사용할 수 있군요!

Comment on lines 93 to 95
font-size: 16px;
line-height: 160%; /* 25.6px */
}
Copy link
Contributor

Choose a reason for hiding this comment

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

line-height를 설정할 때 160% 처럼 % 단위가 아닌 1.6 과 같은 단위 없는 작성법은 어떻게 생각하시나요? 두 개의 차이점은 만약 스타일 상속이 발생했을 때 확인할 수 있어요. 지금 상황에 자식요소가 만약 12px의 폰트 사이즈일 경우를 가정하면, 자식 요소의 line-height도 160%로 계산된 값(25.6px)이 적용되는 반면, 단위 없이 1.6이라고 할 경우 12px의 1.6배인 19.2px이 적용될 거예요. 그 폰트의 크기에 맞게 비율이 조정되는 것이죠.

Comment on lines 114 to 119
font-family: Pretendard, sans-serif;
font-size: 12px;
font-style: normal;
font-weight: 400;
line-height: normal;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

css의 각 속성들은 기본 값을 갖고 있지요. 자주 사용하는 속성들의 기본 값을 알고 있으면 기본 값을 설정하는 상황에서 생략할 수 있을 거예요! 지금 이 코드에서는 font-style, font-weight, line-height를 생략할 수 있을 것 같아요!

Comment on lines 152 to 154
<div
className={`${styles['time-table__node']} ${styles['time-table__node--long']}`}
>
Copy link
Contributor

Choose a reason for hiding this comment

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

cn 유틸을 사용하지 않고 리터럴로 적용한 이유가 무엇인가요?

Comment on lines 108 to 111
<div className={styles['time-table__node-wrapper']}>
<div
className={styles['time-table__node']}
>
Copy link
Contributor

Choose a reason for hiding this comment

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

아주 사소하지만 태그와 속성 사이에 개행을 주고 안 주고 하는 코드 스타일에 일관성이 있으면 좋을 것 같아요! 예를 들어 저는 속성이 3 가지 이상이거나, 두 가지더라도 태그 라인이 길어질 경우에 줄넘김을 넣어주고 있어요!

@junghaesung79 junghaesung79 force-pushed the feat/#604-2/bus-timetable branch from c746af3 to 3dac107 Compare December 31, 2024 06:08
@junghaesung79 junghaesung79 merged commit a62a9b3 into develop Dec 31, 2024
2 checks passed
@github-actions github-actions bot deleted the feat/#604-2/bus-timetable branch December 31, 2024 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[버스] 버스 시간표 페이지 개편
3 participants