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

결과페이지 첫 방문시에만 Tooltip 보이도록 수정 #458

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

sjoleee
Copy link
Member

@sjoleee sjoleee commented Nov 5, 2023

🤔 해결하려는 문제가 무엇인가요?

매번 툴팁이 보여서 불편하다는 유저 피드백 반영

🎉 변경 사항

로컬스토리지에 isRevisit을 추가하고 unmount 시점에 true로 변경해줍니다.
true이면 툴팁을 렌더링하지 않습니다.

더 좋은 방법이 있다면 제보 바랍니다~~

🙏 여기는 꼭 봐주세요!

사용 방법

🌄 스크린샷

📚 참고

@sjoleee sjoleee self-assigned this Nov 5, 2023
Copy link

github-actions bot commented Nov 5, 2023

Bundle Sizes

Compared against 59b9fec

Route: No significant changes found

Dynamic import: No significant changes found

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f80fcd8) 89.38% compared to head (5494366) 89.38%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #458   +/-   ##
=======================================
  Coverage   89.38%   89.38%           
=======================================
  Files          44       44           
  Lines         339      339           
  Branches       62       62           
=======================================
  Hits          303      303           
  Misses         36       36           
Files Coverage Δ
src/constants/storage.ts 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@hyesungoh hyesungoh left a comment

Choose a reason for hiding this comment

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

쥬스쥬스 🧃 🧃 🧃 🧃 👍 👍 👍 👍

@@ -3,4 +3,5 @@ export const LOCAL_STORAGE_KEY = {
reviewShortQuestionMessages: 'nlasq',
surveyCustomQuestions: 'nlascq',
surveyCreateSurveyRequest: 'nlascsr',
resultRevisit: 'nlarv',
Copy link
Member

Choose a reason for hiding this comment

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

👍 👍 👍

@@ -44,6 +47,8 @@ const PILL_COLORS: Color[] = ['bluegreen', 'pink', 'skyblue', 'yellowgreen', 'pu

const SurveyIdLoaded = ({ surveyId }: Props) => {
const { fireToast } = useToast();
const [isRevisit, setIsRevisit] = useLocalStorage<boolean>(`${LOCAL_STORAGE_KEY.resultRevisit}`, false);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const [isRevisit, setIsRevisit] = useLocalStorage<boolean>(`${LOCAL_STORAGE_KEY.resultRevisit}`, false);
const [isRevisit, setIsRevisit] = useLocalStorage<boolean>(LOCAL_STORAGE_KEY.resultRevisit, false);

상수가 문자열이라 이렇게 써도 되지 않을까욘?? 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

오옹 이렇게 쓰면 어떤 변수가 들어와도 문자열임을 보장할 수 있어서 권장하는 방법인가요?! 궁금 @hyesungoh

Copy link
Member

@hyesungoh hyesungoh Nov 6, 2023

Choose a reason for hiding this comment

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

`${LOCAL_STORAGE_KEY.resultRevisit}`

산됴우님이 작성하신 방법은 말씀하신 것처럼 어떤 값이 들어와도 문자열임을 보장할 수 있는 방법인 거 같아용

근데 제가 말씀드린

LOCAL_STORAGE_KEY.resultRevisit

요건 저장된 값이 문자열이니, 그냥 사용해도 되지 않을까 ~~~ 하는 생각입니다 !


resultRevisit: 'nlarv',

Copy link
Collaborator

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.

c9b4833에서 수정 완~~~!

Copy link
Member

@hyesungoh hyesungoh left a comment

Choose a reason for hiding this comment

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

👍 👍 👍 👍 7ㅏ즈아 🦈 🦈 🦈 🦈 🦈 🦈 🦈

@sjoleee sjoleee merged commit 7a1d9f1 into main Nov 7, 2023
11 of 12 checks passed
@sjoleee sjoleee deleted the fix/tooltip-storage branch November 7, 2023 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants