-
Notifications
You must be signed in to change notification settings - Fork 2
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] 게임화면 및 대기화면 기능 구현 #54
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
빠른 작업 처리 너무 좋습니다 포메!!! 닉네임 빼고는 참고사항이라 approve 드릴게요! 참고로 reviewer에 안달아주시면 메일이 안와요ㅜ 그리고 UI 작업이나 컴포넌트 작업을 했을 때는 화면 스크린샷을 찍어주시면 좋을 것 같아요! 수고하셨습니당
|
||
const Content = ({ children }: PropsWithChildren) => { | ||
return <section css={contentSection}>{children}</section>; | ||
return <section css={contentLayout}>{children}</section>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L5 - 참고 의견
layout 명 고치느라 고생많았네요!!! 컨벤션 지키려고 노력하는 거 아주 좋습니당
<div css={mainPageLayout}> | ||
<div css={logoWrapper}>LO to the GO</div> | ||
<h1 css={title}>땅콩</h1> | ||
<div css={intro}>어색한 분위기를 주도해봐요</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L4 - 변경 제안
기본 텍스트에는 span 태그나 h1 다음으로 h2 를 사용하는 것도 좋아보이네요! reset css 를 적용했으니 폰트 크기 외에 태그의 웹 접근성 측면만 고려해도 괜찮을 것 같네용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이미 폰트 사이즈가 적용되어 있으니 굳이 h1을 안써도 될 것 같긴하네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아뇨?? h1은 좋고 div를 span 아니면 h2 로 두면 좋겠다는 의견이였습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아! 아랫 문장 말하는거였군요. 잘못 이해했어용
display: flex; | ||
align-items: center; | ||
width: 26.8rem; | ||
height: 4.9rem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L5 - 참고 의견
이렇게 애매한 숫자가 나온 이유가 있을까요?? 피그마를 그대로 옮긴거라면 디지안할 때 너비랑 높이를 다 지정하면서 하진 않았어서 이런 값이 나온 것 같아요! 4px 단위로 하면 좋을 것 같습니당
import RoundResultPage from '@/pages/RoundResultPage/RoundResultPage'; | ||
|
||
export const router = createBrowserRouter([ | ||
{ | ||
path: '/', | ||
element: <MainPage />, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L5 - 참고 의견
오호 메인페이지만 헤더가 없게 만들 수 있군요! 좋네요 ㅎㅎ
gray200: '#F3F1F1', | ||
gray300: '#E4E4E4', | ||
gray400: '#9D9B9B', | ||
gray500: '#7A7A7A', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L5 - 참고 의견
테마 색상 상수화 아주 좋습니다 ㅎㅎ
frontend/src/utils/nickname.ts
Outdated
@@ -0,0 +1,9 @@ | |||
export const createRandomNickname = () => { | |||
const adjectives = ['배고픈', '성실한', '욕망의', '섹시한', '멋있는', '타락한']; | |||
const nouns = ['마루', '썬데이', '프린', '이든', '포메', '타칸', '커찬']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L4 - 변경 제안
형용사랑 명사 랜덤으로 닉네임 나오는 거 재밌게 좋네요 ㅎㅎ 근데 배포될 것을 고려하면 동물 이름이나 ~땅콩 으로 고정해놓는 건 어떨까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L4 - 변경 제안
const adjectives = [
'춤추는', '노래하는', '웃긴', '귀여운', '사랑스러운', '재치있는',
'기운찬', '활발한', '지적인', '영리한', '장난꾸러기', '유쾌한',
'깔끔한', '매력적인', '화려한', '신나는', '용감한', '상냥한',
'달콤한', '기발한', '짜릿한', '다정한', '평온한', '쾌활한',
'따뜻한', '호기심 많은', '재빠른', '천재적인', '엉뚱한', '호탕한',
'멋진', '섹시한', '발랄한', '당당한', '명랑한', '흥미로운'
];
저도 여러개 가져왔습니다 푸하하 부정적인 단어는 빼고 조금 더 다양하게 넣어보았어요. 이렇게 다양하게 되면 nonus는 땅콩으로 고정되어도 될 것 같고요. 다들 어떻게 생각하시는지 ?
@@ -1,7 +1,7 @@ | |||
import { buttonLayout } from './Button.styled'; | |||
|
|||
interface ButtonProps { | |||
text: '선택' | '확인' | '다음'; | |||
text: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L5 - 참고 의견
저도 수정하면서 좀 고민이 되었는데 버튼 내의 텍스트는 그냥 string으로 열어둬도 괜찮을 것 같긴 하네용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
폼에 ~ 빠른 구현 너무 좋네요 👍🚀 몇 가지 코멘트 달았는데 확인해 주시고 함께 이야기 나눠 보아요 ~! ! ! 👏
const randomNickname = createRandomNickname(); | ||
|
||
return ( | ||
<Content> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L5 - 참고 의견
알아서 찰떡 같이 공통 Content layout 잘 적용하신 모습을 보니 뿌듯하네요 ✨
<div css={profile}></div> | ||
<div css={nickname}>닉네임</div> | ||
<div css={nicknameInputWrapper}> | ||
<input css={nicknameInput} type="text" placeholder={randomNickname} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L4 - 변경 제안
저희 근데 어차피 지금은 닉네임을 변경할 수 없으니 placeholder로 안 하고 value에 넣어도 되지 않을까 하는 생각도 있는데, 어떻게 생각하시나요 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
value랑 placeholder 둘 다 변경이 가능해서 기능 상의 차이는 없지 않을까 생각합니다!
frontend/src/router/index.tsx
Outdated
element: <div>게임 대기 화면</div>, | ||
}, | ||
{ | ||
path: 'game/:roundId', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L3 - 중요 질문
저희 path에 roundId를 보여주지 않기로 협의한 것 같아서 이야기 나눠보고 path를 수정해야할 것 같아요 ~!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 그렇네요! roundId는 없는 게 맞을 것 같아요
gray200: '#F3F1F1', | ||
gray300: '#E4E4E4', | ||
gray400: '#9D9B9B', | ||
gray500: '#7A7A7A', | ||
} as const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L5 - 참고 의견
찰떡같이 peanut과 맞춘 200, 300, 400, 500 형식 너무 좋네요 ^^
frontend/src/utils/nickname.ts
Outdated
@@ -0,0 +1,9 @@ | |||
export const createRandomNickname = () => { | |||
const adjectives = ['배고픈', '성실한', '욕망의', '섹시한', '멋있는', '타락한']; | |||
const nouns = ['마루', '썬데이', '프린', '이든', '포메', '타칸', '커찬']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L4 - 변경 제안
const adjectives = [
'춤추는', '노래하는', '웃긴', '귀여운', '사랑스러운', '재치있는',
'기운찬', '활발한', '지적인', '영리한', '장난꾸러기', '유쾌한',
'깔끔한', '매력적인', '화려한', '신나는', '용감한', '상냥한',
'달콤한', '기발한', '짜릿한', '다정한', '평온한', '쾌활한',
'따뜻한', '호기심 많은', '재빠른', '천재적인', '엉뚱한', '호탕한',
'멋진', '섹시한', '발랄한', '당당한', '명랑한', '흥미로운'
];
저도 여러개 가져왔습니다 푸하하 부정적인 단어는 빼고 조금 더 다양하게 넣어보았어요. 이렇게 다양하게 되면 nonus는 땅콩으로 고정되어도 될 것 같고요. 다들 어떻게 생각하시는지 ?
Issue Number
#46
As-Is
메인 화면과 닉네임 생성 화면이 없던 상황
To-Be
메인 화면 및 방 만들기 기능
닉네임 입력 및 랜덤 닉네임 생성
Check List
Test Screenshot
(Optional) Additional Description