Add ToggleEmojiButtonGroup and ToggleEmojiButtonSource component
#2301
Conversation
🦋 Changeset detectedLatest commit: f7a9162 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
Chromatic Report🚀 Congratulations! Your build was successful! |
yangwooseong
force-pushed
the
WEB-1983/toggle-icon-button
branch
from
509911c
to
ba77c42
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2301 +/- ##
==========================================
- Coverage 83.28% 82.27% -1.01%
==========================================
Files 141 143 +2
Lines 2943 2979 +36
Branches 899 904 +5
==========================================
Hits 2451 2451
- Misses 487 523 +36
Partials 5 5 ☔ View full report in Codecov by Sentry. |
yangwooseong
force-pushed
the
WEB-1983/toggle-icon-button
branch
from
ba77c42
to
bd7e934
Compare
|
요거는 이모지 기획 변경 & 적용되면 다시 리뷰할게요 |
yangwooseong
force-pushed
the
WEB-1983/toggle-icon-button
branch
3 times, most recently
from
8bb023f
to
1c44d57
Compare
| /** | ||
| * If `loading` is true, spinner will be shown, replacing the content. | ||
| * @default false | ||
| */ | ||
| loading?: boolean |
There was a problem hiding this comment.
마이너) 토글 버튼 전반에 스펙에는 존재하지 않는 loading prop을 미리 만들어둘 필요가 있을까? 생각이 문득 들었습니다 🤔 사용할 일이 거의 없을 거 같아요
There was a problem hiding this comment.
그렇네요 저도 지워도 될것같습니다
| <ToggleGroup.Root | ||
| type="single" | ||
| defaultValue={defaultValue} | ||
| onValueChange={onValueChange} | ||
| value={value} | ||
| > |
There was a problem hiding this comment.
ToggleGroup.Root 가 div를 렌더하고 있기 때문에, DOM 크기를 줄이기 위해 추가 div없이 이를 컨테이너로 활용하는게 좋을 거 같아요.
| const mergedRefs = useMergeRefs(ref, forwardedRef) | ||
| const { fillDirection } = useToggleEmojiButtonContext() | ||
|
|
||
| const { adjustButtonSize } = useResizeButton({ button: ref.current }) |
There was a problem hiding this comment.
지금 구조에선 버튼 컴포넌트마다 컨테이너에 ResizeObserver를 붙이는데, 컨테이너당 하나의 ResizeObserver만 붙이도록 개선할 수 있을 거 같습니다.
useResizeButton (useEffect)를 컨테이너에서만 호출하고, 버튼의 사이즈(React state)를 반환하도록 한 뒤에 이를 컨텍스트를 통해 전달하면 될 거 같아요. 어차피 동일한 컨테이너의 모든 버튼은 동일한 사이즈를 가질거니까요.
아래는 간단하게 작성한 예시입니다..!
interface UseResizeButtonProps {
container: HTMLElement | null
childrenSize: number
}
// NOTE: 네이밍은 적절히 수정
export function useResizeButton({
container,
childrenSize,
}: UseResizeButtonProps) {
const [buttonSize, setButtonSize] = useState<number>(EMOJI_BUTTON_SIZE)
const { fillDirection } = useToggleEmojiButtonContext()
const adjustButtonSize = useCallback(() => {
if (!container || fillDirection !== 'all') {
return
}
const containerWidth = container.clientWidth
const containerHeight = container.clientHeight
const size = Math.max(
Math.min(
(containerWidth - EMOJI_BUTTON_GROUP_GAP * (childrenSize - 1)) /
childrenSize,
containerHeight - EMOJI_BUTTON_GROUP_GAP
),
EMOJI_BUTTON_SIZE
)
setButtonSize(size)
}, [childrenSize, container, fillDirection])
/* ... */
return buttonSize
}/* container */
const buttonSize = useResizeButton({
container: ref,
childrenSize: React.Children.count(children),
})
/* button */
const { buttonSize, fillDirection } = useToggleEmojiButtonContext()
<BaseButton
ref={mergedRefs}
// onResize={handleResize}
style={
{
'--b-toggle-emoji-button-size': cssDimension(EMOJI_BUTTON_SIZE),
'--b-toggle-emoji-button-emoji-size': cssDimension(EMOJI_SIZE),
width:
fillDirection === 'all' ? cssDimension(buttonSize) : undefined,
height:
fillDirection === 'all' ? cssDimension(buttonSize) : undefined,
...style,
} as CSSProperties
}There was a problem hiding this comment.
생각해보니 컨테이너에서 컨텍스트로 주지 않고 컨테이너의 css variable로 전달해주기만 해도 되겠네요..!
|
|
||
| const { adjustButtonSize } = useResizeButton({ button: ref.current }) | ||
|
|
||
| const handleResize: ReactEventHandler<HTMLButtonElement> = (e) => { |
There was a problem hiding this comment.
이 이벤트 핸들러가 꼭 필요한가요? 컨테이너의 리사이즈 이벤트 핸들러에서 처리해줄 거로 보여서요!
…zontal in storybook
yangwooseong
force-pushed
the
WEB-1983/toggle-icon-button
branch
from
00f80f0
to
cfbe5e7
Compare
| const shouldResizeButton = fillDirection === 'all' | ||
| const resizedButtonSize = useResizeButton({ | ||
| container: ref, | ||
| enable: shouldResizeButton, |
There was a problem hiding this comment.
| enable: shouldResizeButton, | |
| enabled: shouldResizeButton, |
(마이너)
| }, | ||
| forwardedRef | ||
| ) { | ||
| const [ref, setRef] = useState<null | HTMLDivElement>(null) |
There was a problem hiding this comment.
| const [ref, setRef] = useState<null | HTMLDivElement>(null) | |
| const [container, setContainer] = useState<null | HTMLDivElement>(null) |
(마이너) React RefObject가 아니라서 이 네이밍이 더 적절해보여요
| buttonCount: number | ||
| } | ||
|
|
||
| export function useResizeButton({ |
There was a problem hiding this comment.
(마이너) useToggleEmojiButtonSize 같은 이름은 어떤가요? 버튼 사이즈를 반환한다는 점이 더 잘 드러난다고 생각해요. 지금 이름은 아무것도 반환하지 않을 거 같은 뉘앙스가 있는 거 같아요..!
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @channel.io/[email protected] ### Minor Changes - The use client directive has been added at the top of all components inside @channel.io/bezier-react. ([#2468](#2468)) by @nayounsang ### Patch Changes - Remove 1px gap between icon and text in `s` size `AlphaButton`, `AlphaFloatingButton`. ([#2484](#2484)) by @yangwooseong - Apply hovered color token for `AlphaButton`, `AlphaFloatingButton`, and `AlphaFloatingIconButton`. ([#2471](#2471)) by @yangwooseong - Add `ToggleEmojiButtonGroup` and `ToggleEmojiButtonSource` component. ([#2301](#2301)) by @yangwooseong - Updated dependencies - @channel.io/[email protected] ## @channel.io/[email protected] ### Patch Changes - Change `alpha-color-green-300-0` alpha token. ([#2471](#2471)) by @yangwooseong Modify hovered color generating formula so that they are more visible in dark theme. ## @channel.io/[email protected] ### Patch Changes - Updated dependencies - @channel.io/[email protected] ## [email protected] ### Patch Changes - Updated dependencies - @channel.io/[email protected] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Self Checklist
Related Issue
Summary
ToggleEmojiButtonGroup과ToggleEmojiButtonSource컴포넌트를 구현합니다.Details
ToggleEmojiButtonSource컴포넌트안에 이모지를 보여줄때, imageUrl를 props로 열여주는 것도 생각했지만 이렇게 하면 모바일에서 구현이 어려운 문제가 있어서 content속성을 그대로 활용했습니다.Breaking change? (Yes/No)
References