Skip to content

Conversation

@annie1229
Copy link
Contributor

@annie1229 annie1229 commented Jan 16, 2023

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.
  • [Component] I wrote a unit test about the implementation.
  • [Component] I wrote a storybook document about the implementation.
  • [Component] I tested the implementation in various browsers.
    • Windows: Chrome, Edge, (Optional) Firefox
    • macOS: Chrome, Edge, Safari, (Optional) Firefox
  • [New Component] I added my username to the correct directory in the CODEOWNERS file.

Related Issue

Fixes #629

Summary

  • Text 컴포넌트에 truncated 속성을 추가합니다.

Details

  • 현재 베지어 Text 컴포넌트는 span(inline 엘리먼트)을 사용하고 있습니다. inline 엘리먼트에서는 text-overflow 속성을 제대로 적용할 수 없습니다.(inline 엘리먼트는 너비를 가지지 않고 컨텐츠 크기만큼 공간을 차지함)

    The text-overflow property only affects content that is overflowing a block container element in its inline progression direction

    따라서, truncated 옵션을 주었을 때 text-overflow가 제대로 적용될 수 있도록 display: block 속성도 추가하였습니다.

  • (고민되는 부분) display: block 속성을 추가하는 것에 있어 block이라는 prop을 하나 더 열어주고, 말줄임 속성을 적용하고 싶으면 block, truncated를 둘 다 적용하는 방식이 더 명확하게 display 속성이 block으로 변화되는 것을 알 수 있다는 생각이 들었습니다.
    (Text의 display: flex 삭제 #487 에서 예기치 못하게 inline -> block 속성으로 변경되어서 문제가 된 적이 있기 때문에)
    하지만 사용처에서 항상 block, trucated를 같이 적용해야한다는 불편함과, truncated 단독으로 옵션을 주었을 때는 말줄임이 제대로 적용되지 않기 때문에 같이 스타일링 하는 것으로 작성하였습니다.

    1. 리서치 결과 span(inline 엘리먼트)과 함께 truncated 속성을 사용하는 경우에는 display: block or inline-block이 포함된 경우가 많음
    2. 말줄임 속성이 적용되는 곳은 inline -> block 속성으로 바뀌어도 문제가 없을 것으로 예상됨

    다만, 현재 Text props 중 marginTop, marginBottom, marginVertical 속성은 별도의 display: block or inline-block스타일링을 주고있지 않다면 margin이 제대로 적용되지 않고 있기 때문에, block prop을 하나 더 열어주고 필요할 때 사용해도 좋을것 같다는 생각입니다.

  • truncated 스타일에 display: block 이 포함되지 않은 사례

  • truncated 스타일에 display: block 이 포함된 사례

  • 데스크에 적용된 후에, 후속으로 데스크 Text ellipsis 관련 스타일링 코드를 마이그레이션 할 예정입니다.

Breaking change or not (Yes/No)

No

References

@annie1229 annie1229 added enhancement Issues or PR related to making existing features better component labels Jan 16, 2023
@annie1229 annie1229 self-assigned this Jan 16, 2023
@changeset-bot
Copy link

changeset-bot bot commented Jan 16, 2023

🦋 Changeset detected

Latest commit: 152ee52

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

@github-actions
Copy link
Contributor

github-actions bot commented Jan 16, 2023

Chromatic Report

🚀 Congratulations! Your build was successful!

@codecov
Copy link

codecov bot commented Jan 16, 2023

Codecov Report

Base: 72.61% // Head: 72.60% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (152ee52) compared to base (b509000).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           next-v1    #1103      +/-   ##
===========================================
- Coverage    72.61%   72.60%   -0.01%     
===========================================
  Files          237      237              
  Lines         3137     3136       -1     
  Branches       853      854       +1     
===========================================
- Hits          2278     2277       -1     
  Misses         728      728              
  Partials       131      131              
Impacted Files Coverage Δ
...rc/components/Forms/Inputs/Select/Select.styled.ts 95.23% <ø> (-0.22%) ⬇️
...eact/src/components/Forms/Inputs/Select/Select.tsx 92.72% <ø> (ø)
.../KeyValueListItem/common/KeyItem/KeyItem.styled.ts 100.00% <ø> (ø)
...onents/KeyValueListItem/common/KeyItem/KeyItem.tsx 100.00% <ø> (ø)
.../src/components/Modals/LegacyModal/Modal.styled.ts 100.00% <ø> (ø)
...src/components/Modals/LegacyModal/ModalContent.tsx 80.00% <ø> (ø)
...c/components/Navigator/NavGroup/NavGroup.styled.ts 100.00% <ø> (ø)
...act/src/components/Navigator/NavGroup/NavGroup.tsx 70.58% <ø> (ø)
...src/components/Navigator/NavItem/NavItem.styled.ts 100.00% <ø> (ø)
...react/src/components/Navigator/NavItem/NavItem.tsx 83.33% <ø> (ø)
... and 2 more

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@annie1229 annie1229 changed the title Add truncated prop to Text component Add truncated prop to Text component Jan 16, 2023
@annie1229 annie1229 marked this pull request as ready for review January 17, 2023 02:31
@Dogdriip Dogdriip self-requested a review January 19, 2023 06:40
@sungik-choi
Copy link
Contributor

채널 웹 데스크 어플리케이션에 로컬 빌드 적용하셔서 테스트 한 번 부탁드립니다!

Copy link
Contributor

@guswnsxodlf guswnsxodlf left a comment

Choose a reason for hiding this comment

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

truncate 여부에 상관 없이 block 혹은 inline-block을 주는 방법은 안되는 걸까요?

@annie1229
Copy link
Contributor Author

annie1229 commented Jan 26, 2023

채널 웹 데스크 어플리케이션에 로컬 빌드 적용하셔서 테스트 한 번 부탁드립니다!

웹 데스크에서 Text + ellipsis() 사용처에서 전반적으로 확인해본 결과 잘 적용됩니다!
(테스트에 어려움이 있는 비즈니스인증, 밋, 주문연동 쪽은 아직 테스트 못함)

@annie1229
Copy link
Contributor Author

annie1229 commented Jan 26, 2023

truncate 여부에 상관 없이 block 혹은 inline-block을 주는 방법은 안되는 걸까요?

#487 에서 예기치 못하게 inline -> block 속성으로 변경되어서 문제가 되었던 히스토리가 있어서,
Text에 truncate 여부와 상관없이 block 혹은 inline-block을 주는것은 비슷한 문제가 발생할 수도 있지 않을까 생각합니다~

})

it('Text receives bold style', () => {
it('Text receives italic style', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@annie1229 annie1229 requested a review from guswnsxodlf February 9, 2023 09:11
@annie1229 annie1229 merged commit 3bdb95e into channel-io:next-v1 Feb 14, 2023
@annie1229 annie1229 deleted the enhance/text-add-truncated branch February 14, 2023 01:15
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

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

Add truncated prop to Text component

4 participants