QA 이슈 수정 #3

junghaesung79 · 2024-07-12T02:43:08Z

Close [영양사] 프로덕션 QA fix #2

What is this PR? 🔍

기능 : QA 이슈 수정
issue : [영양사] 프로덕션 QA fix #2

Changes 📝

미운영 필터링
사파리에서 품절모달 글자 크기
윈도우 품절 모달 이상
스와이프 뒤로갈 때 로그인 화면
품절 모달 백그라운드로 끄기
pdf 제한 <- 잘 되어 있네요. ㅎ; 내가 올렸던게 pdf가 아니었던..
로그인 유지
모바일 페이지 스크롤 막기
삼성브라우저 다크모드
내정보 삭제
문의 폼 링크 수정

ScreenShot 📷

Test CheckList ✅

test 1
test 2
test 3

Precaution

✔️ Please check if the PR fulfills these requirements

It's submitted to the correct branch, not the develop branch unconditionally?
If on a hotfix branch, ensure it targets main?
There are no warning message when you run yarn lint

- ISSUE_NUMBER 글자 삭제

D0Dam

확인했습니다! 수고하셨습니다~!

D0Dam

소소한 리뷰 남겨 보았습니다!

D0Dam · 2024-07-12T02:53:57Z

index.html

@@ -9,6 +9,7 @@
      content="backoffice for koin's coop members"
    />
    <meta name="google" content="notranslate">
+    <meta name="color-scheme" content="light only">


D0Dam · 2024-07-12T02:54:48Z

src/layout/Copyright/Copyright.module.scss

  font-size: 12px;
+  font-weight: 400;


default weight 가 400 아닌가요?

맞아요. 하지만 저는 명시적으로 적는 방법을 고수해요. 피그마에서 서식 복붙함으로써 적용할 수 있다는 편함도 있고 다른 사람이 봤을 때도 명확하게 적혀 있는 게 더 의도를 알기 쉽다고 생각해요. 그리고 혹시 모를 브라우저 기본 스타일이 달랐을 때의 문제도 피할 수 있구요.

피그마의 서식으로 넣는것이 컨벤션은 아닌 것으로 알고 있어요~! 오히려 불필요하게 default 값을 명시하면 그만큼 line 도 너무 늘어나구요!
그리고 해당 옵션으로 인해 브라우저 기본 스타일이 바뀔 수 있나요..?

D0Dam · 2024-07-12T02:55:12Z

src/layout/Copyright/Copyright.module.scss

  font-size: 12px;
+  font-weight: 400;
+  line-height: 19.2px;


픽셀 단위에 대해서 UI/UX 와 이야기를 좀 해보아야 되겠군요..

D0Dam · 2024-07-12T02:55:33Z

src/layout/Copyright/Copyright.module.scss

@@ -1,11 +1,13 @@
 @use "src/assets/styles/mediaQuery" as media;

 .copyright {
+  font-family: Pretendard, sans-serif;


전역 폰트 패밀리 설정이 안되어 있을까요?

네 Pretendard 폰트는 현재 전역설정이 없습니다.

D0Dam · 2024-07-12T02:56:18Z

src/layout/Header/MobilePanel/index.tsx


  const {
    isExpanded: isMobileSidebarExpanded,
    expandSidebar,
    hideSidebar,
  } = useMobileSidebar(pathname, isMobile);

+  const handleHamburgerClick = () => {
+    navigate(pathname, { replace: true });


D0Dam · 2024-07-12T03:00:08Z

src/pages/Login/Login.module.scss

    margin-top: 16px;
-    position: relative;
-    width: inherit;
+    padding: 0 16px;


tmi : padding-inline 이라는걸로 써도 된다.

오.. 잊고 있던 키워드 알려주셔서 감사해요!

D0Dam · 2024-07-12T03:00:47Z

src/pages/Login/Login.module.scss

+    background-color: #f5f5f5;
+    display: flex;
+    align-items: center;
+    flex-direction: row;


이것도 default flex direction 같군요.

flex 부분도 다른 사람이 읽었을 때 명시되어 있는 게 좋다고 생각하여 적어줍니다.

저와 같은 경우에는 명시를 해주지 않아도 당연히 알고 넘어간다고 생각합니다! box-sizing, position 같은 다른 css 속성 처럼요.

D0Dam · 2024-07-12T03:02:34Z

src/pages/Login/index.tsx

+  const [id, setId] = useState<string>('');
+  const [password, setPassword] = useState<string>('');


초기에 id, password가 string인 것이 맞을까요?
string | null 과는 차이가 없을까요?

html에서 input 값에 아무 값도 설정하지 않으면 그것은 빈 문자열로 된다고 알고 있습니다. input의 type이 text이기도 하구요. 그래서 저는 ''을 초기 값으로 두고 타입도 string으로 해도 문제 없다고 생각합니다. 혹시 다른 의견 있으시면 말씀해주세요!

오호 좋은 생각이에요! 저도 이견 없습니다!👍

D0Dam · 2024-07-12T03:03:51Z

src/query/auth.ts

-          default:
-            errorMessage = '로그인을 실패했습니다.';
-            sendClientError(err);
+        if (![400, 404, 403, 500].includes(err.status)) {


해당 배열을 상수화해서 네이밍으로 의미를 부여해주면 더 좋을 것 같네요!

89eaec2
COOP_LOGIN_ERROR_CODES로 선언했습니다!

junghaesung79 and others added 13 commits July 11, 2024 09:29

refactor: 사용하지 않는 훅 삭제

0e48efd

refactor: 햄버거 메뉴에서 스와이프 뒤로 가기 문제

c764647

refactor: 미운영 식단 숨기기

fbd6b0a

fix: token refresh 로직 수정 및 적용

9c919b6

feat: 모달 닫기 기능 및 접근성 추가

772cf7b

feat: useIsomorphicLayoutEffect, useScroll 훅 추가 및 적용

f7159c5

refactor: 로그인 input 스타일 수정

c8f8ddf

refactor: AuthLayout default background color 고정

b28469c

refactor: 로그인 키 활/비활 추가 및 에러메시지 로직 변경

16bf93f

refactor: 오늘 날짜 표시 바 높이 수정

fbc34fd

refactor: 다크모드 제한

b0a2263

refactor: pull request 템플릿 수정

ffab131

- ISSUE_NUMBER 글자 삭제

junghaesung79 added the ❗QA issue ❗QA 이슈 label Jul 12, 2024

junghaesung79 self-assigned this Jul 12, 2024

github-actions bot requested review from dooohun and KimKyungYun July 12, 2024 02:43

D0Dam self-requested a review July 12, 2024 02:48

D0Dam self-assigned this Jul 12, 2024

D0Dam linked an issue Jul 12, 2024 that may be closed by this pull request

[영양사] 프로덕션 QA fix #2

Closed

D0Dam approved these changes Jul 12, 2024

View reviewed changes

junghaesung79 added 2 commits July 12, 2024 13:19

refactor: 에러 코드 배열화

89eaec2

refactor: 문의 폼 링크 수정

d3e134c

junghaesung79 merged commit cd38683 into develop Jul 12, 2024
1 check passed

github-actions bot deleted the 2-production-qa-fix branch July 12, 2024 11:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QA 이슈 수정 #3

QA 이슈 수정 #3

junghaesung79 commented Jul 12, 2024 •

edited

Loading

D0Dam left a comment

D0Dam left a comment

D0Dam Jul 12, 2024

D0Dam Jul 12, 2024

junghaesung79 Jul 12, 2024

D0Dam Jul 12, 2024

D0Dam Jul 12, 2024

D0Dam Jul 12, 2024

junghaesung79 Jul 12, 2024

D0Dam Jul 12, 2024

D0Dam Jul 12, 2024

junghaesung79 Jul 12, 2024

D0Dam Jul 12, 2024

junghaesung79 Jul 12, 2024

D0Dam Jul 12, 2024

D0Dam Jul 12, 2024

junghaesung79 Jul 12, 2024

D0Dam Jul 12, 2024

D0Dam Jul 12, 2024

junghaesung79 Jul 12, 2024

		const [id, setId] = useState<string>('');
		const [password, setPassword] = useState<string>('');

QA 이슈 수정 #3

QA 이슈 수정 #3

Conversation

junghaesung79 commented Jul 12, 2024 • edited Loading

What is this PR? 🔍

Changes 📝

ScreenShot 📷

Test CheckList ✅

Precaution

✔️ Please check if the PR fulfills these requirements

D0Dam left a comment

Choose a reason for hiding this comment

D0Dam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junghaesung79 commented Jul 12, 2024 •

edited

Loading