channel-io / bezier-react

React components library that implements Bezier design system.
https://main--62bead1508281287d3c94d25.chromatic.com
Apache License 2.0
181 stars 45 forks source link

SectionLabel의 help Tooltip의 allowHover를 true로 설정 #815

Closed Dogdriip closed 1 year ago

Dogdriip commented 1 year ago

Summary

SectionLabel 컴포넌트의 help 프롭 타입에 allowTooltipHover: boolean을 추가하여, SectionLabel을 사용할 때 Help tooltip의 allowHover 여부를 정할 수 있게 합니다.

SectionLabel 컴포넌트의 help 툴팁의 allowHover를 항상 true로 변경합니다.

Details

관련 Desk 이슈: https://github.com/channel-io/ch-desk-web/issues/10023

Help tooltip 내에 Clickable한 요소가 있는 경우 등에는 Tooltip이 호버 가능해야 하며, 이를 SectionLabel.help의 props에 추가하여 해결합니다. SectionLabel.help를 렌더링하는 툴팁의 allowHover를 true로 설정하여 해결합니다.

Browser Compatibility

OS / Engine 호환성을 반드시 확인해주세요.

Windows

References

codecov[bot] commented 1 year ago

Codecov Report

Merging #815 (85677ef) into next-v1 (cf8a976) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff            @@
##           next-v1     #815   +/-   ##
========================================
  Coverage    67.81%   67.81%           
========================================
  Files          204      204           
  Lines         2862     2862           
  Branches       784      784           
========================================
  Hits          1941     1941           
  Misses         804      804           
  Partials       117      117           
Impacted Files Coverage Δ
...react/src/components/SectionLabel/SectionLabel.tsx 97.29% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 844e77c...85677ef. Read the comment docs.

Dogdriip commented 1 year ago

prop 추가가 아니라 기본적으로 allowHover = true로 하는 것은 어떻게 생각하시나요?

반대로 생각했을 때(SectionLabel의 help에 대해 툴팁 allowHover를 원치 않는 경우)가 약간 걸리긴 하지만, 딱히 툴팁 hover를 막아야 할 케이스가 떠오르지 않네요 😅 저는 괜찮을 것 같습니다!

Dogdriip commented 1 year ago

덧붙여, (지금 받는 tooltipContent도 어차피 TooltipProps에 들어가기 때문에) 다음과 같이 tooltipProps를 대신 받아서 이를 spread해주고, tooltipContent를 deprecated 처리하는 방안도 생각해 보았습니다. 사실 위의 논의대로 allowHover를 무조건 true로 주면 해결되는 문제이지만, 이렇게 하면 어떨지 의견 주시면 감사하겠습니다 🙇

As-is:

interface SectionLabelHelpProps extends Partial<IconInfo> {
  iconSize?: IconSize
  tooltipContent: React.ReactNode
}

To-be:

interface SectionLabelHelpProps extends Partial<IconInfo> {
  iconSize?: IconSize
  tooltipProps: Partial<TooltipProps> & Pick<TooltipProps, 'content'>

  /**
   * @deprecated
   */
  tooltipContent: React.ReactNode
}
inhibitor1217 commented 1 year ago

내부 컴포넌트의 prop을 지정하는 선택지를 늘리고 싶다면, 이를 인터페이스에 추가하기보다는 합성을 통해 해결하는 것이 좋아보입니다.

interface SectionLabelProps {
  ...

  tooltip: { /* preset props for tooltip */ } | React.ReactNode
}
quino0627 commented 1 year ago

저 리뷰리퀘가 너무많아서 뺐습니다~ 🙏

ch-builder commented 1 year ago

:tada: This PR is included in version 1.0.0-next-v1.148 :tada:

The release is available on:

Your semantic-release bot :package::rocket: