fix: truncated text when width is auto #1673
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dotaitch
reviewed
May 17, 2022
| const context = document.createElement('canvas').getContext('2d')!; | ||
| context.font = font; | ||
| const { width } = context.measureText(String(text)); | ||
| export function getTextWidth(text: string, font?: string) { |
There was a problem hiding this comment.
font 인자값이 쓰이지 않네요. 추후 개발을 위해 font를 나두어야 한다면 아래 eslint 룰을 적용해도 좋을 것 같아요. 참고로 TOAST UI Calendar에도 적용되어 있습니다.
https://eslint.org/docs/rules/no-unused-vars#argsignorepattern
adhrinae
approved these changes
May 18, 2022
Comment on lines
320
to
322
| const bodyArea = document.querySelector( | ||
| `.${cls('rside-area')} .${cls('body-container')} .${cls('table')}` | ||
| ); |
There was a problem hiding this comment.
bodyArea 를 인자로 받게 할 수는 없을까요? 인자를 넘기는게 코드가 좀 복잡해보이긴 하지만, 디스패치에서 이 함수를 루프에서 직접 호출하는게 너무 큰 사이드 이펙트라고 느껴져서요. 어차피 그리드가 마운트되고 나면 이 엘리먼트는 어디로 사라지는 것도 아니니까 reduce 루프의 상위 스코프에서 한 번만 찾고 인자로 전달하는 식으로 했음 좋겠네요.
lja1018
approved these changes
May 18, 2022
Comment on lines
447
to
449
| return texts.reduce((acc, text) => { | ||
| return text.length > acc.length ? text : acc; | ||
| }, ''); |
There was a problem hiding this comment.
Suggested change
| return texts.reduce((acc, text) => { | |
| return text.length > acc.length ? text : acc; | |
| }, ''); | |
| return texts.reduce((acc, text) => text.length > acc.length ? text : acc, ''); |
바로 리턴할 수 있을 것 같아요.
jwlee1108
approved these changes
May 19, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please check if the PR fulfills these requirements
fix #xxx[,#xxx], where "xxx" is the issue number)Description
As-Is
To-Be
Thank you for your contribution to TOAST UI product. 🎉 😘 ✨