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

[Feat/#91] SelectStationModal 컴포넌트 구현 #96

Merged
merged 6 commits into from
Jan 16, 2025

Conversation

thisishwarang
Copy link
Collaborator

📌 관련 이슈번호


체크리스트

  • 🎋 base 브랜치를 develop 브랜치로 설정했나요?
  • 🖌️ PR 제목은 형식에 맞게 잘 작성했나요?
  • 🏗️ 빌드는 성공했나요? (yarn build)
  • 🧹 불필요한 코드는 제거했나요? e.g. console.log
  • 🙇‍♂️ 리뷰어를 지정했나요?
  • 🏷️ 라벨은 등록했나요?

✅ Key Changes

이번 PR에서 작업한 내용을 간략히 설명해주세요

  1. 지하철역을 선택하는 SelectStationModal 컴포넌트를 구현했습니다.
    • viewPage에서는 현재 선택된 지하철역을 저장하고 있어야 하기 때문에 currentLocation 상태를 저장합니다.
    • 이를 수정하는 handle함수를 모달로 넘겨줍니다.
  2. SelectStationModal 컴포넌트에서는 현재 임의로 지하철역 배열을 만들어 놨지만 추후에는 api로 get해올 예정입니다.
    • 모달 내부에서 선택된 지하철역을 저장하기 위해 selectedLocation state를 만들었습니다.
      이 state가 없을 시 지하철역을 선택할 때마다 currentLocation이 수정되어야 하는 불편함이 있기 때문에 이를 만들었습니다.

📢 To Reviewers


📸 스크린샷 or 실행영상

image

Copy link
Collaborator

@youtheyeon youtheyeon left a comment

Choose a reason for hiding this comment

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

스크롤 바 위치 잡는 게 어려웠을 텐데 넘 잘하셨네요 고생하셨습니다!
수정 사항 확인 부탁드려요 🥺

export const modalLayoutStyle = style([
flexGenerator('column', 'flex-start', 'flex-start'),
{
maxWidth: 430,
Copy link
Collaborator

Choose a reason for hiding this comment

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

전역으로 변수 설정해줬으니까 maxWidth: 'var(--max-width)', 이런 식으로 쓰는 게 더 좋을 것 같습니다!

gap: '1rem',
paddingLeft: '2rem',
width: '100%',
maxHeight: 'calc(100dvh - 8rem - 7.3rem - 4rem - 6rem)',
Copy link
Collaborator

Choose a reason for hiding this comment

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

아니 이것뭐예요

'::-webkit-scrollbar-thumb': {
backgroundColor: vars.colors.gray300,
borderRadius: 50,
border: '6px solid white',
Copy link
Collaborator

Choose a reason for hiding this comment

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

white?????????????

setCurrentLocation(location);
};

console.log(currentLocation);
Copy link
Collaborator

Choose a reason for hiding this comment

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

콘솔 지워주세요:)!

import { Modal } from '@components';
import { useModal } from '@hooks';
import { SelectStationModal } from '@pages/view/components';
import { useState } from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

import { useState } from 'react'; 가 맨 위로 오게.. 하는 게 더 자연스러워 보이는데 어떨까요..?

};
return (
<>
<Header />
Copy link
Collaborator

@youtheyeon youtheyeon Jan 15, 2025

Choose a reason for hiding this comment

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

아래 레이아웃으로 깔리는 헤더도 z-index가 1이고 여기에서 불러오는 헤더도 1일 텐데 괜찮을까요..?

@@ -91,7 +91,7 @@ export const vars = createGlobalTheme(':root', {
},
head09_r_18: {
fontWeight: '400',
fontSize: '2rem',
fontSize: '1.8rem',
Copy link
Collaborator

Choose a reason for hiding this comment

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

레전드 매의 눈

export const footerSection = style({
width: '100%',
marginTop: 'auto',
padding: '0 2rem',
Copy link
Collaborator

Choose a reason for hiding this comment

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

피그마에 있는 패딩 값이랑 좀 다른 것 같습니다.. 🥹 확인 부탁드려요!

Comment on lines 12 to 13
maxHeight: '5.6rem',
minHeight: '5.6rem',
Copy link
Collaborator

Choose a reason for hiding this comment

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

flexShrink: 0으로 주면 height이 고정될 것 같습니다!

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

🍰 Storybook 배포가 완료되었습니다! 🔗 https://677cb5c3ab50c3f524eaae19-yyazkbksmq.chromatic.com/

Copy link
Collaborator

@zzz-myam zzz-myam left a comment

Choose a reason for hiding this comment

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

@youtheyeon 님의 공격적인 코리에 큰 공감을 하며, 저는 부드럽게 어푸푸 남기고 갑니다. (꼼꼼하게 다 봤습니다 진짜요)

Copy link
Collaborator

@chaeneey chaeneey left a comment

Choose a reason for hiding this comment

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

아침부터 열심히 만드신 그 모달 .. 잘봤습니다 .. 스크롤가지 .. 굿^^

Copy link

🍰 Storybook 배포가 완료되었습니다! 🔗 https://677cb5c3ab50c3f524eaae19-ghosuwyxcv.chromatic.com/

Copy link

🍰 Storybook 배포가 완료되었습니다! 🔗 https://677cb5c3ab50c3f524eaae19-fpiduevgxz.chromatic.com/

@thisishwarang thisishwarang merged commit 41287aa into develop Jan 16, 2025
3 checks passed
@thisishwarang thisishwarang deleted the feat/#91/SelectStationModal branch January 16, 2025 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 지하철역 선택 모달 컴포넌트 구현
4 participants