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 대응 - 로그인 리다이렉트 및 데탑 뷰 대응 #402

Merged
merged 13 commits into from
Feb 16, 2025

Conversation

seong-hui
Copy link
Member

@seong-hui seong-hui commented Feb 9, 2025

작업 내용 🧑‍💻

  • 로그인한 유저일 경우 /today로 리다이렉트 시키는 로직 추가하였습니다.
  • 디자인이 1920px * 1080px로 이루어져 대부분의 작업 환경인 맥북 13인치와 맞지 않아 font-size를 45%로 수정하고 height는 view height 기준으로 늘어날 수 있게 하였습니다.
  • 쏟아내기 on/off에 따라서 캘린더의 width를 남은 화면을 꽉 채울 수 있도록 수정하였습니다.
  • todo의 진행 상태를 수정하는 토글 영역 밖을 누르면 해당 모달이 꺼지도록 수정하였습니다.
  • 모달의 마감기간 터치영역이 아이콘으로 한정되어 있어서 해당 터치영역을 넓혔습니다.
  • 캘린더의 종일 부분의 테두리가 약간 맞지 않아서 해당 부분도 함께 수정했습니다.
  • 캘린더 상단 버튼들이 일직선으로 있지 않아서 해당 부분 스타일도 수정하였습니다.

알게된 점 🚀

기록하며 개발하기!

리뷰 요구사항 💬

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

  • 로그인 리다이렉트 관련 브랜치였으나 화면 크기가 맞지 않는게 너무 거슬려 함께 작업하였습니다 죄송함다.
  • 간만에 한 캘린더 css 커스텀은 역시나 힘들었슴다. 코드가 몇개 수정되지 않았지만 찾는데 상당한 시간이 들었습니다 ,,
  • 기본 폰트가 너무 작다고 생각되시면 말씀해주세요!!

관련 이슈

close #401

스크린샷 (선택)

  • 캘린더 상단 버튼 정렬
image

@TEAM-DAWM TEAM-DAWM deleted a comment from github-actions bot Feb 9, 2025
@TEAM-DAWM TEAM-DAWM deleted a comment from github-actions bot Feb 9, 2025
@seong-hui seong-hui requested a review from jeeminyi February 10, 2025 08:40
Copy link
Member

@wrryu09 wrryu09 left a comment

Choose a reason for hiding this comment

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

어우 css 고생많으셨습니다 👍
브라우저 크기대응되는거 속이 다 시원하네욤 ㅋㅋㅋ

Comment on lines 149 to 151
html {
font-size: 62.5%;
font-size: 45%;
}
Copy link
Member

Choose a reason for hiding this comment

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

얘가 45%가 되면 기본 폰트 크기가 10px이 되는 62.5와 다르게 피그마 보고 rem 단위로 css 작성하기가 번거로울 것 같은데, 좋은 대비책이 있을까요? 기존에 작성한 값들 기준으로 가서 별 상관없나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

기존에 모든 폰트 크기를 rem 단위로 작성했기 때문에 font-size의 % 값을 조정하는 것이 비율적으로 큰 영향을 주지는 않겠지만, 현재 디자인이 1920px * 1080px을 기준으로 제작되었기 때문에 font-size의 % 값을 줄이지 않으려면 피그마에서 설정된 디자인 크기 자체를 수정하는 것이 필요할 것으로 보입니다..!
디자인 크기의 즉각적인 수정이 어렵기 때문에, 우선 임시 방편으로 font-size: 45%로 설정해두었습니다.

Comment on lines 12 to 19
const isAuthenticated = localStorage.getItem('accessToken');
const naviate = useNavigate();

useEffect(() => {
if (isAuthenticated) {
naviate('/today');
}
}, [isAuthenticated, naviate]);
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.

해당 기능 useAuthRedirect 훅으로 분리했습니다! 훨씬 코드가 깔끔해지고 좋으네용 좋은 의견 감사합니다!!

Copy link
Contributor

@Kjiw0n Kjiw0n left a comment

Choose a reason for hiding this comment

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

로그인 리다이렉트 잘 작동되는 것 확인해습니다!! 굳굳 👍 👍

다만 제 화면에서 이렇게 보이는 이슈(글씨 넘넘 작아짐) & 10px=1rem 대응 이슈로 font-size는 62.5% 기준으로 두고 가는 게 좋을 것 같아요..! 반응형 처리 디쟌이랑 다시 얘기해봐야할 것 같네요
스크린샷 2025-02-10 23 42 13

+캘린더 월간 쪽은 다른 브랜치에서 처리하는 걸로 할까요? 현재는 적용 안된 상태로 보입니다!
스크린샷 2025-02-10 23 44 43

@seong-hui
Copy link
Member Author

image font-size를 62.5%로 둘 경우 맥북 에어 13인치 defult 해상도 환경(1440 * 900)에서 위와 같이 폰트가 너무 크게 나타나서
	@media (width <= 1440px) {
		html {
			font-size: 50%;
		}
	}

이와 같이 미디어 커리를 사용해서 1440px이하에서만 font-size를 줄이는 방식은 어떨까요?

Copy link

@TEAM-DAWM TEAM-DAWM deleted a comment from github-actions bot Feb 16, 2025
@seong-hui seong-hui merged commit 6931c09 into develop Feb 16, 2025
4 checks passed
@seong-hui seong-hui deleted the feat/#401/login-redirect branch February 16, 2025 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FEAT] 로그인한 유저일 경우 메인 페이지로 리다이렉트 기능 추가
3 participants