-
Notifications
You must be signed in to change notification settings - Fork 72
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: Handle spaces in initialConsonants search inputs properly #42
fix: Handle spaces in initialConsonants search inputs properly #42
Conversation
🦋 Changeset detectedLatest commit: 6798078 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for es-hangul ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 고생하셨습니다 🙇
@BO-LIKE-CHICKEN ...는 Conflict가 발생했군요 😢 |
64afb00
to
63beabb
Compare
@evan-moon 충돌을 해결했습니다! #36 과 충돌이 있었네요 🥲 다시 리뷰해주시면 감사하겠습니다! 다만, #41 이슈에서 @okinawaa 님과 함수의 역할에 대해 논의하기 이전의 PR이므로, 병합 시 문제가 될 가능성이 있습니다. 두 리뷰어의 의견을 모아주시면 좋을 것 같아요. 🙏🏻 해당 이슈에 따른 개선안으로도 PR을 생성할 예정입니다. 감사합니다! |
src/chosungIncludes.ts
Outdated
return false; | ||
} | ||
|
||
const initialConsonantsX = getFirstConsonants(x).replace(/\s/g, ''); | ||
const initialConsonantsY = getFirstConsonants(y).replace(/\s/g, ''); | ||
const initialConsonantsY = trimY; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위에서 초성으로만 구성되었는지 확인하기 때문에 getFirstConsonants
는 필요없습니다.
참고사항
감사합니다! |
Co-authored-by: 박찬혁 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
참고사항으로 남겨주신 두 가지 의견모두 동의합니다! 넘 좋네용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰를 너무 늦게해서 죄송합니다
감사해요!
Overview
#41 에서 발견한 '공백이 포함된 초성 검색 시 예상치 못한 false 반환 문제'를 공백 포함 입력도 정상적으로 처리하도록 수정했습니다.
PR Checklist