Skip to content

Conversation

@sookyeonghwang
Copy link
Member

작업 내용

  • fix: 검색 input 클릭 시 placeholder 제거
  • fix: 검색 최대 12글자까지 검색할 수 있도록 수정

참고 이미지(선택)

어떤 점을 리뷰 받고 싶으신가요?

@netlify
Copy link

netlify bot commented Feb 19, 2023

Deploy Preview for record-it-dev ready!

Name Link
🔨 Latest commit 9a99933
🔍 Latest deploy log https://app.netlify.com/sites/record-it-dev/deploys/63f204d79c53280008042d3f
😎 Deploy Preview https://deploy-preview-229--record-it-dev.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@Seongtaek-H Seongtaek-H left a comment

Choose a reason for hiding this comment

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

확인했습니다.
마이레코드에서 처음에 검색 결과 페이지로 이동하려면 무조건 엔터키를 눌러야하네요? 저는 change 이벤트 발생하면 디바운스 적용 후 바로 이동하는 것으로 생각했는데 정책서랑 TC를 봐도 좀 애매하긴하네요.

Copy link
Contributor

@endmoseung endmoseung left a comment

Choose a reason for hiding this comment

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

확인했습니다!

setIsClickedInput,
...props
}: SearchInputProps) {
const KEYWORD_MAX_LENGTH = 12
Copy link
Contributor

Choose a reason for hiding this comment

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

저희 이 12는 모두가 하나의 상수로 두고 써야될것같아요
예를들어 우리가 12> 15로 정책이 바뀌면 레코드 추가페이지에서도 수정해야하고, 워터페이지에도 수정해야하니 하나의 상수만 변경해도 모두가 반영되도록하는게 유지보수에 유리할거같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

넵넵 :) 반영해놓을게요!

@sookyeonghwang
Copy link
Member Author

확인했습니다. 마이레코드에서 처음에 검색 결과 페이지로 이동하려면 무조건 엔터키를 눌러야하네요? 저는 change 이벤트 발생하면 디바운스 적용 후 바로 이동하는 것으로 생각했는데 정책서랑 TC를 봐도 좀 애매하긴하네요.

오늘 QA 때 얘기해보고 변경된다면 배포 전에 반영해보도록 하겠습니다 :)

@sookyeonghwang sookyeonghwang merged commit 4fc9511 into develop Feb 19, 2023
@sookyeonghwang sookyeonghwang deleted the FE-224 branch February 19, 2023 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐞 Bug 버그

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants