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] CompletePage 사용자 만족도 조사 추가 #343

Merged
merged 10 commits into from
Aug 5, 2024

Conversation

lydiacho
Copy link
Member

@lydiacho lydiacho commented Aug 2, 2024

Related Issue : Closes #341


🧑‍🎤 Summary

Complete Page에 사용자 만족도 조사를 추가합니다

🧑‍🎤 Screenshot

2024-08-03.12.24.32.mov

🧑‍🎤 Comment

  • 만족도 조사 추가되면서 레이아웃 전반적으로 좁아져서 수정사항 반영했어요

🚀 조건부 렌더링되는 컴포넌트에 애니메이션 넣기

일반적으로 조건부 렌더링은 JSX에서 조건부로 다른 컴포넌트 리턴시켜주는 식으로 구현되는데요,
이번에 점수선택부분소중한의견감사드려요 텍스트는 transition 효과가 들어가야 했어요.
근데 일반적인 방법대로 조건부렌더링 해주면 fade-out 애니메이션이 보여지기도 전에 컴포넌트가 언마운트 돼서 애니메이션 효과가 적용이 안됩니다. (아마 언석님도 겪어보셨을 이슈일거예요)
따라서

  • 두 컴포넌트를 감싸는 div를 하나더 만들어서 position: relative를 주고
  • 각 컴포넌트에 position: absolute 를 줘서 위치를 겹쳐준 다음
  • styleVariant를 통해 opacity 값을 조건부 스타일링 해주었습니다.

이런 케이스에, 언석님이 사용하시는 다른 해결책이 있다면 그것도 궁금해요!! 검색해봐도 다 별로 내키지 않더라구요..

참고로 큰 역할 안하는 태그에는 그냥 인라인 스타일링 해주었는데, 혹시 인라인 스타일 모두 스타일파일에 분리시켜주길 원하시면 말씀해주세요!! 반영하겠습니다

🚀 텍스트 세로 정렬 대체 어떻게 해결하시나요
텍스트 세로 중앙 정렬....
이것도 항상 겪었던 이슈인데 명쾌한 해답이 없는걸까요
늘 그래왔던 것처럼 이번에도 텍스트를 감싸는 span 태그에 어림잡아 paddingTop: 5px 만 줬는데,
이것도 언석님만의 해결책이 있는지 궁금합니다
검색하면 항상 line-height 같은거 추천해주는데 하나도 도움 안되더라고요 ㅜ

참고로 API 연결은 내일 찬기오빠가 작업해주면 이후에 붙여야 하는데,
어차피 API 코드는 복잡하지 않고 똑같을 것 같아서 일단 퍼블리싱 먼저 코리 받고,
API 코드 추가로 작업하면 확인차 태그 한번 드릴게요 !

@auto-assign auto-assign bot requested a review from eonseok-jeon August 2, 2024 15:45
Copy link

height bot commented Aug 2, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link
Member

@eonseok-jeon eonseok-jeon left a comment

Choose a reason for hiding this comment

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

두 컴포넌트를 감싸는 div를 하나더 만들어서 position: relative를 주고
각 컴포넌트에 position: absolute 를 줘서 위치를 겹쳐준 다음
styleVariant를 통해 opacity 값을 조건부 스타일링 해주었습니다.

좋은 거 같습니다!! 저도 보통 이런 경우 opacity 0, 1을 이용해주는 편이라서요

참고로 큰 역할 안하는 태그에는 그냥 인라인 스타일링 해주었는데, 혹시 인라인 스타일 모두 스타일파일에 분리시켜주길 원하시면 말씀해주세요!! 반영하겠습니다

큰 상관 없는 거 같아여~ 너무 길어지지 않은 한 편한대로 해주세여 :)

텍스트 세로 중앙 정렬....
이것도 항상 겪었던 이슈인데 명쾌한 해답이 없는걸까요
늘 그래왔던 것처럼 이번에도 텍스트를 감싸는 span 태그에 어림잡아 paddingTop: 5px 만 줬는데,
이것도 언석님만의 해결책이 있는지 궁금합니다
검색하면 항상 line-height 같은거 추천해주는데 하나도 도움 안되더라고요 ㅜ

어떤 걸 말씀하는 건지 조금 이해가 안 됩니다ㅜ
paddingTop: 5 없애도 큰 차이를 못 느끼겠어요,,!
1, 2, 3 얘네랑 소중한 의견 감사합니다 이 친구의 텍스트 높이를 일치 시키고 싶으시단 건가요?
그렇다면 지금처럼 paddingTop을 주면 될 거 같아여 :)
lineHeight이랑 paddingTop 말고는 딱히 방법이 생각나진 않네여

고생하셨습니다 :)

src/views/CompletePage/index.tsx Outdated Show resolved Hide resolved
src/views/CompletePage/index.tsx Outdated Show resolved Hide resolved
@lydiacho
Copy link
Member Author

lydiacho commented Aug 3, 2024

@eonseok-jeon

  • API 연결 완
    • 응답 받을 필요도 없고 성공/에러 처리할 것도 없어서 정말 간단하게 만들었어요
  • 코드리뷰 반영 모두 완
  • 이거 언석님은 이렇게 안보이시나요?? 전 makers 라이팅 때문에 저렇게 두줄로 나눠져버려서 전체 width 제한 늘려주었습니다!
스크린샷 2024-08-04 오전 1 23 34

(후)

스크린샷 2024-08-04 오전 1 28 33

마지막으로 PR에 말씀드린 line-height 이슈는 아래 영상 참고해주세요!! 우선 paddingTop 넣는게 최선인 것 같아 보이네요 ㅠㅠ

2024-08-04.1.29.46.mov

Copy link
Member

@eonseok-jeon eonseok-jeon left a comment

Choose a reason for hiding this comment

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

고생하셨ㅅ습니ㅏㄷ :)

src/views/CompletePage/types.ts Outdated Show resolved Hide resolved
Comment on lines 13 to 21
mutate({ satisfaction: i });
setPoint(i);
setTimeout(() => {
setPoint('CHANGED');
}, 200);
setTimeout(() => {
setPoint(-1);
}, 2500);
};
Copy link
Member

Choose a reason for hiding this comment

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

이렇게 하면 네트워크가 꺼져 있어서 실제로 api 요청이 가지 않아도 전송이 완료 된 거 처럼 돼요

api 요청이 성공 시에(onSuccess) 해당 로직이 반영되게 하면 좋을 거 같아요

그렇게 하려면 요청 보낼 때 loading 문구도 표시해주면 좋을 거 같습니다 :)

Copy link
Member Author

Choose a reason for hiding this comment

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

그럼 로딩 디자인을 다시 요청드리는 것보다
'소중한 의견 감사합니다' 를 요청 성공 이후 2초 후에 띄우는게 좋을 것 같은데 어떻게 생각하시나요?

Copy link
Member

@eonseok-jeon eonseok-jeon Aug 4, 2024

Choose a reason for hiding this comment

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

음? 왜죠? 성공 메세지는 성공하면 바로 띄우는 게 좋지 않나요?

파일 업로드와 같이 업로드 중일 때 업로드 중입니다 문구가 뜨고, 완료 되면 바로 파일 이름이 뜨는 것처럼요

Copy link
Member Author

@lydiacho lydiacho Aug 4, 2024

Choose a reason for hiding this comment

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

대부분 네트워크 통신은 곧바로 성공할텐데 디자인 측에서는 2초 정도 지연 후에 라이팅 보여지길 원하셔서요 -> 아 2초 아니고 0.2초요 ㅠㅠ (기존에 있던 지연 유지)

Copy link
Member

Choose a reason for hiding this comment

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

아항 고렇군요 좋습니다~

Copy link
Member Author

Choose a reason for hiding this comment

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

반영해두었습니다! 확인 후 따봉 눌러주시면 머지할게요!

@lydiacho lydiacho merged commit f93aa11 into develop Aug 5, 2024
@lydiacho lydiacho deleted the feat/#341_user-survey branch August 5, 2024 04:51
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] 지원 완료 페이지에 만족도 조사 추가
2 participants