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

[Feat] 숫자 + 코, 숫자 + 단 입력시 텍스트에 스타일 적용되도록 #26

Merged
merged 10 commits into from
Apr 15, 2021

Conversation

YuuRiLee
Copy link
Contributor

@YuuRiLee YuuRiLee commented Apr 3, 2021

PR 제안 사유

숫자 + 코, 숫자 + 단 입력시 텍스트에 스타일 적용되도록 합니다

Resolve #25

주요 변경 기록

  • draft.js, @draft-js-plugins/editor package를 추가했습니다
  • 숫자 + 단위를 입력시 스타일이 커스텀되는 UnitDecorator plugin을 추가했습니다

11코 + 스페이스 => 스타일된 11코
11코 + 11코 => 스페이스로 구분하지 않으면 스타일 적용 안됨
가나다코 => 숫자 + 단위로 입력해야 함
11코 + 코 지우기 => 숫자 + 단위 형식이 깨짐으로 스타일이 적용 안됨
스타일 적용된 숫자 + 단위 클릭시 스타일이 토글됨

plugin 만드는데 참고한 코드는 다음과 같습니다

Code review

Code review 에서 중점적으로 봐야하는 부분

사용자가 편할지

Design review

Design review 에서 중점적으로 봐야하는 부분 / 스크린샷

스크린샷 2021-04-04 오전 5 03 33

화면 기록 2021-04-04 오전 5 10 44 mov

기타 질문 및 특이 사항

@YuuRiLee YuuRiLee added Feat 새로운 기능과 관련된 작업 Design 디자인과 관련된 작업 High 우선순위 상 labels Apr 3, 2021
@YuuRiLee YuuRiLee self-assigned this Apr 3, 2021
Copy link
Contributor

@kimdoori kimdoori left a comment

Choose a reason for hiding this comment

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

와우 수고 많으셨어요! 한율님 코멘트랑 겹치는 건 이모지만 찍고 추가로 달진 않았어요!!

src/plugins/unitDecorator/unitDecorator.tsx Outdated Show resolved Hide resolved
@zoripong
Copy link
Member

zoripong commented Apr 4, 2021

사용성 코멘트

  • 클릭했는데 placeholder가 안사라져요 👀
  • 1코2코 입력하면 1코는 계산 되는 코로 인식되는데 맞나요?
  • 계산되는 코를 hover 했을 때 액션을 취할 수 있다는 건 알것 같은데 어떤 액션이 일어날지 예상할수가 없군요 🤔
  • 계산된 코를 클릭해서 풀었을 때 rollback 할 방법이 없네요..
  • 엔터치니까... 갑자기 계산된 코로 변경되네요! ref
  • 근데 도안 작성시 코를 안 붙이는 사람도 있어서 걱정이네용 🤔
  • 딱히 액션 없이 n코 입력하면 자동으로 계산된 코로 변경되는건 편하네요! 안번거롭고!

이건 지금 당장 다 고쳐야 하는건 아니고 확인해달라고 해서 확인하고 드는 생각 코멘트로 남긴겁니다 ㅎㅎ

@YuuRiLee
Copy link
Contributor Author

YuuRiLee commented Apr 4, 2021

  • 클릭했는데 placeholder가 안사라져요 👀

클릭하고 값을 입력해야 합니다

  • 1코2코 입력하면 1코는 계산 되는 코로 인식되는데 맞나요?

가운데 공백이 없으면 계산되는 코가 아니어야 할텐데 말이죠 🤔

  • 계산되는 코를 hover 했을 때 액션을 취할 수 있다는 건 알것 같은데 어떤 액션이 일어날지 예상할수가 없군요 🤔

어떤 방식으로 풀면 좋을까요

  • 계산된 코를 클릭해서 풀었을 때 rollback 할 방법이 없네요..

다시 클릭하면 됩니담

  • 엔터치니까... 갑자기 계산된 코로 변경되네요! ref

확인해봐야겠네요

  • 근데 도안 작성시 코를 안 붙이는 사람도 있어서 걱정이네용 🤔

하지만 코를 안 붙이면 이게 단인지 코인지 알 수 있는 방법이 없을 것 같아요

@zoripong
Copy link
Member

zoripong commented Apr 4, 2021

클릭하고 값을 입력해야 합니다

원래 placeholder가 입력하면 사라지는군요..! 근데 placeholder 뒤에 커서가 잡히던데 이건 MUI가 그렇게 제공하는건가요?

어떤 방식으로 풀면 좋을까요

tooltip 밖에 떠오르는게 없군요 🤔 🤔

다시 클릭하면 됩니담
앗 그러네.. ㅋㅋ 이건 진짜 알길이 없군요... 마우스 가져다 대는거 아니면.. 🤔
계산 안하는 코의 경우에도 블럭(?)으로 감싸고 색깔만 다르게해야하나 싶네요..

하지만 코를 안 붙이면 이게 단인지 코인지 알 수 있는 방법이 없을 것 같아요

기본적으로 코를 많이 얘기해서.. 도안 읽는 사람은 코와 단이 없어도 이해할 수 있는데... 저희가 계산이 불가능하겠네요 ㅠㅠ 어쩔수 없을 것 같아요

@YuuRiLee
Copy link
Contributor Author

YuuRiLee commented Apr 6, 2021

클릭하고 값을 입력해야 합니다

원래 placeholder가 입력하면 사라지는군요..! 근데 placeholder 뒤에 커서가 잡히던데 이건 MUI가 그렇게 제공하는건가요?

아니요 여기 input에서는 MUI 안써요 draft-js Editor 컴포넌트 씁니다 draft-js 문제인 것 같군요 🤔

어떤 방식으로 풀면 좋을까요

tooltip 밖에 떠오르는게 없군요 🤔 🤔

다시 클릭하면 됩니담
앗 그러네.. ㅋㅋ 이건 진짜 알길이 없군요... 마우스 가져다 대는거 아니면.. 🤔
계산 안하는 코의 경우에도 블럭(?)으로 감싸고 색깔만 다르게해야하나 싶네요..

약간 disabled 느낌을 주면 좋을 것 같군욤

하지만 코를 안 붙이면 이게 단인지 코인지 알 수 있는 방법이 없을 것 같아요

기본적으로 코를 많이 얘기해서.. 도안 읽는 사람은 코와 단이 없어도 이해할 수 있는데... 저희가 계산이 불가능하겠네요 ㅠㅠ 어쩔수 없을 것 같아요

@YuuRiLee
Copy link
Contributor Author

YuuRiLee commented Apr 6, 2021

@zoripong @kimdoori [Refactor: unitDecorator unit defaultValue "코"로 수정](Refactor: unitDecorator unit defaultValue "코"로 수정)부터 봐주세요!

Copy link
Contributor

@kimdoori kimdoori left a comment

Choose a reason for hiding this comment

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

굳 리뷰 반영 감사합니다!!
너무 늘어지는 것 같아서.. (제송..)고칠 부분 있다면 추후에 리팩토링하는 것도 좋을 것 같아요~~

Copy link
Member

@zoripong zoripong left a comment

Choose a reason for hiding this comment

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

수고많으셨어용 최고!

@YuuRiLee YuuRiLee force-pushed the stitche-and-row-custom-style branch from bf2ed24 to 806721a Compare April 11, 2021 05:17
@YuuRiLee
Copy link
Contributor Author

@kimdoori 포푸해당 커밋 봐주세요
@zoripong 포푸만 봐주세요

Copy link
Contributor

@kimdoori kimdoori left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@zoripong zoripong left a comment

Choose a reason for hiding this comment

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

백점 만점 십만점

@YuuRiLee YuuRiLee merged commit 418990f into main Apr 15, 2021
@YuuRiLee YuuRiLee deleted the stitche-and-row-custom-style branch April 15, 2021 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design 디자인과 관련된 작업 Feat 새로운 기능과 관련된 작업 High 우선순위 상
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 숫자 + 코, 숫자 + 단 입력시 텍스트에 스타일 적용되도록
3 participants