Skip to content

Conversation

@yangwooseong
Copy link
Contributor

@yangwooseong yangwooseong commented Sep 5, 2022

Self Checklist

  • I wrote a PR title in English.
  • I added an appropriate label to the PR.
  • I wrote a commit message in English.
  • I wrote a commit message according to the Conventional Commits specification.
  • I added the appropriate changeset for the changes.
  • I wrote a unit test about the implementation.
  • I wrote a storybook document about the implementation.
  • I tested the implementation in various browsers.
    • Windows: Chrome, Edge, (Optional) Firefox
    • macOS: Chrome, Edge, Safari, (Optional) Firefox

Related Issue

Fixes #922

Summary

  • Change word-break property to break-word in Modal component

Details

Breaking change or not (Yes/No)

  • No

References

@yangwooseong yangwooseong added enhancement Issues or PR related to making existing features better fix PR related to bug fix labels Sep 5, 2022
@yangwooseong yangwooseong self-assigned this Sep 5, 2022
@changeset-bot
Copy link

changeset-bot bot commented Sep 5, 2022

🦋 Changeset detected

Latest commit: e408cd3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@channel.io/bezier-react Patch
bezier-figma-plugin Patch

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

@codecov
Copy link

codecov bot commented Sep 5, 2022

Codecov Report

Merging #924 (e408cd3) into next-v1 (b559bd1) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           next-v1     #924   +/-   ##
========================================
  Coverage    70.87%   70.87%           
========================================
  Files          204      204           
  Lines         2884     2884           
  Branches       797      797           
========================================
  Hits          2044     2044           
  Misses         722      722           
  Partials       118      118           
Impacted Files Coverage Δ
...-react/src/components/Modals/Modal/Modal.styled.ts 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2022

Chromatic Report

🚀 Congratulations! Your build was successful!

@yangwooseong yangwooseong force-pushed the fix/remove-break-all-property-in-modal branch from 663c733 to 12549af Compare September 5, 2022 07:40
@yangwooseong yangwooseong changed the title Fix word-break style of Modal content Change word-break style of Modal content from break-all to break-word Sep 5, 2022
@yangwooseong yangwooseong requested review from Seolhun, quino0627 and sungik-choi and removed request for Seolhun, quino0627 and sungik-choi September 5, 2022 07:48
@yangwooseong yangwooseong marked this pull request as ready for review September 5, 2022 07:48
@sungik-choi
Copy link
Contributor

한글이나 일본어에 대해선 문제 없을까요?

@yangwooseong
Copy link
Contributor Author

yangwooseong commented Sep 6, 2022

한글이나 일본어에 대해선 문제 없을까요?

네 한글이나 일본어에 대해서는 음절단위로 줄 바꿈이 됩니다. 한 가지 약간 걸리는 것은 mdn에서는 word-break: break-word 가 deprecated 되어서(단 여전히 지원이 되고 있긴 함), 이거 대신 word-break: normaloverflow-wrap: anywhere를 조합해서 쓰라고는 하더라고요. 근데 overflow-wrap: anywhere 가 호환성이 좋지 않아서 word-break: break-word를 쓰는 게 좋을 것 같다는 생각입니다.

image

https://developer.mozilla.org/en-US/docs/Web/CSS/word-break
https://caniuse.com/?search=overflow-wrap%3A%20anywhere

@sungik-choi sungik-choi removed the fix PR related to bug fix label Sep 6, 2022
@sungik-choi
Copy link
Contributor

sungik-choi commented Sep 6, 2022

한글이나 일본어에 대해선 문제 없을까요?

네 한글이나 일본어에 대해서는 음절단위로 줄 바꿈이 됩니다. 한 가지 약간 걸리는 것은 mdn에서는 word-break: break-word 가 deprecated 되어서(단 여전히 지원이 되고 있긴 함), 이거 대신 word-break: normaloverflow-wrap: anywhere를 조합해서 쓰라고는 하더라고요. 근데 overflow-wrap: anywhere 가 호환성이 좋지 않아서 word-break: break-word를 쓰는 게 좋을 것 같다는 생각입니다.

image

https://developer.mozilla.org/en-US/docs/Web/CSS/word-break https://caniuse.com/?search=overflow-wrap%3A%20anywhere

overflow-wrap: break-word 는 어떤가요? anywhere 는 사파리에서 지원을 하지 않아서 사용할 수 없을 거 같고, 일반적인 상황에서 anywhere 와 대체로 같아 보입니다. word-wrapoverflow-wrap 의 alias인데, 해당 속성으로 소스 파일에서 검색해보시면 사용처가 꽤 있어서 한 번 조사해보시면 좋을 거 같습니다!

(FYI. 이 포스트 좋네요. https://wit.nts-corp.com/2017/07/25/4675)

@yangwooseong
Copy link
Contributor Author

한글이나 일본어에 대해선 문제 없을까요?

네 한글이나 일본어에 대해서는 음절단위로 줄 바꿈이 됩니다. 한 가지 약간 걸리는 것은 mdn에서는 word-break: break-word 가 deprecated 되어서(단 여전히 지원이 되고 있긴 함), 이거 대신 word-break: normaloverflow-wrap: anywhere를 조합해서 쓰라고는 하더라고요. 근데 overflow-wrap: anywhere 가 호환성이 좋지 않아서 word-break: break-word를 쓰는 게 좋을 것 같다는 생각입니다.
image
https://developer.mozilla.org/en-US/docs/Web/CSS/word-break https://caniuse.com/?search=overflow-wrap%3A%20anywhere

overflow-wrap: break-word 는 어떤가요? anywhere 는 사파리에서 지원을 하지 않아서 사용할 수 없을 거 같고, 일반적인 상황에서 anywhere 와 대체로 같아 보입니다. word-wrapoverflow-wrap 의 alias인데, 해당 속성으로 소스 파일에서 검색해보시면 사용처가 꽤 있어서 한 번 조사해보시면 좋을 거 같습니다!

(FYI. 이 포스트 좋네요. https://wit.nts-corp.com/2017/07/25/4675)

overflow-wrap 을 desk나 beizer 에서 검색해봤는데 쓰고 있는 곳은 없는 것 같네요. word-wrap 속성을 쓰고 있는 것 같습니다. 그리고, overflow-wrap: break-word로 했을 떄는 아래와 같은 상황에서 width 가 늘어나버리게 되는 문제가 있습니다.

image

@yangwooseong
Copy link
Contributor Author

솔이 퇴사하셔서 어프루브가 하나 더 필요하네요. @Dogdriip @inhibitor1217 부탁드립니다.

@yangwooseong yangwooseong merged commit 2050b66 into channel-io:next-v1 Sep 14, 2022
@yangwooseong yangwooseong deleted the fix/remove-break-all-property-in-modal branch September 14, 2022 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Issues or PR related to making existing features better

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ModalContent 와 ModalChildrenContent 의 word-break 속성 변경

5 participants