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

Fix : QA 내용 반영하여 수정 #26

Merged
merged 19 commits into from
Aug 24, 2023
Merged

Fix : QA 내용 반영하여 수정 #26

merged 19 commits into from
Aug 24, 2023

Conversation

guesung
Copy link
Collaborator

@guesung guesung commented Aug 24, 2023

💡 왜 PR을 올렸나요?

  • 리팩토링

💁 무엇이 어떻게 바뀌나요?

  1. QA내용들 바탕으로 에러들을 수정하였습니다. 수정된 디자인이 아직 나오지 않아 디자인 부분 제외하고 구현하였습니다.

Screenshot 2023-08-24 at 12 27 15 PM
Screenshot 2023-08-24 at 12 27 24 PM
Screenshot 2023-08-24 at 12 27 32 PM
2. tooltip상태를 localStorage에 저장하여 다른 곳에 다녀와도 상태가 저장되도록 구현하였습니다.

📆 작업 예정인 것이 있나요 ?

💬 리뷰어분들께

@guesung guesung added the 🛠 Fix 버그 수정 label Aug 24, 2023
@guesung guesung requested a review from seondal as a code owner August 24, 2023 03:27
@github-actions github-actions bot added the Fix label Aug 24, 2023
@github-actions
Copy link

Labeler가 제목과 설명에 있는 특별한 텍스트와 일치하는 레이블을 적용했습니다.
Label을 검토하고 필요한 변경 사항을 적용해 주세요.

@guesung guesung removed the request for review from seondal August 24, 2023 03:28
@guesung guesung assigned guesung and unassigned seondal Aug 24, 2023
Copy link
Member

@seondal seondal left a comment

Choose a reason for hiding this comment

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

작업 엄청 빠르시네요.... 크으.. 👍🏻
수고 너무 많으셨습니다아!
피그마에 업데이트 된 사항만 반영하구 바로 머지하면 될 것 같습니다아!
어푸르브 해놓을게요 ☺️

@@ -21,13 +21,14 @@ export default function TalkSection() {
const { isLoading, startLoading } = useLoading({
loadingDelay: 3000,
onStopLoading: () => data && setTalkWord(data.poseWord.content),
initialState: false,
});
const [talkWord, setTalkWord] = useState<string>('포즈로 말해요');
Copy link
Member

Choose a reason for hiding this comment

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

피그마 보시면 텍스트 변경되어 있어요!
포즈로 말해요 -> 제시어에 맞춰\n포즈를 취해요!

console.log(tooltipOpened);
const [isOpen, setIsOpen] = useState<boolean>(true);
useEffect(() => {
if (localStorage.getItem('tooltipIsOpen') === 'false') setIsOpen(false);
Copy link
Member

Choose a reason for hiding this comment

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

전역 상태가 아닌 로컬 스토리지를 사용하신 이유가 있을까요 ?!

@@ -5,7 +5,7 @@ import { Spacing } from '@/components/Spacing';
export default function Talk() {
return (
<div>
<Spacing size={30} />
<Spacing size={80} />
Copy link
Member

Choose a reason for hiding this comment

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

피그마에 8/24 업데이트에 여백 80 -> 64로 변경되었습니당 !

https://www.figma.com/file/bbpetgbWeKNFSQYdfrV39P?node-id=28:403&mode=dev#534945193

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아하 넵 !!

반영 했습니다 👍

6428268

@@ -51,7 +51,7 @@ export default function RootLayout({ children }: { children: React.ReactNode })
<html lang="ko">
<body className="flex min-h-[100vh] w-screen touch-none justify-center bg-slate-100 py-px">
<RecoilContextProvider>
<div className="w-full max-w-440 bg-white text-primary drop-shadow-2xl">
<div className="w-full max-w-440 bg-white text-primary">
Copy link
Member

Choose a reason for hiding this comment

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

shadow가 있는게 입체적인 느낌(?) 도 들고 좋은데.... 문제가 된 drop-shadow 말고 그림자 효과 줄 수 있으면 좋을 것 같네요 ㅠ_ㅠ

별개로 이슈 말씀만 드렸는데 생각지도 못한 부분이 원인이라는걸 발견해주신 규성님 통찰력... 짱....! 👍🏻🙌

@@ -14,12 +15,14 @@ export default function Button({
...props
}: StrictPropsWithChildren<ButtonProps>) {
return (
<button
<motion.button
Copy link
Member

Choose a reason for hiding this comment

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

motion.button 은 처음 보는 것 같은데 .. 배워갑니다아..!

@@ -16,7 +16,7 @@ export default function Header({
...props
}: HeaderProps) {
return (
<div className="fixed inset-x-0 top-0 z-10 bg-white pt-8">
<div className="fixed inset-x-0 top-0 z-10 mx-auto max-w-440 bg-white pt-8">
Copy link
Member

Choose a reason for hiding this comment

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

fixed inset-x-0 인데 max-w 를 넣은 이유가 있을까요 ?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed는 브라우저 전체 사이즈를 기준으로 위치를 잡기에 너비 지정이 필요합니다. 440px보다 작은 디바이스에서는 사이즈가 줄어들 수 있게 max-w로 지정해주었습니다 !

@@ -8,7 +8,7 @@ interface PopupProps {
export default function Popup({ children }: StrictPropsWithChildren<PopupProps>) {
return (
<ModalWrapper>
<section className="flex w-300 flex-col items-center rounded-16 bg-white px-16 py-12">
<section className="flex w-300 flex-col items-center rounded-16 bg-white px-16 py-12">
Copy link
Member

Choose a reason for hiding this comment

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

엇 여기 공백 하나가 들어갔습니다..!

@guesung guesung merged commit 322686b into develop Aug 24, 2023
1 check passed
@guesung guesung deleted the OZ-74-F-QA branch August 24, 2023 10:27
@guesung guesung removed the Fix label Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠 Fix 버그 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants