Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: InformationPanelの内部ロジックをリファクタリングする #5289

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
'use client'

import React, { FC, PropsWithChildren, useCallback, useEffect, useId, useState } from 'react'
import React, {
FC,
PropsWithChildren,
useCallback,
useEffect,
useId,
useMemo,
useState,
} from 'react'
import { VariantProps, tv } from 'tailwind-variants'

import { Base, BaseElementProps } from '../Base'
Expand All @@ -12,7 +20,7 @@ import { ResponseMessage } from '../ResponseMessage'

import type { DecoratorsType } from '../../types'

type Props = PropsWithChildren<{
type AbstractProps = PropsWithChildren<{
/** パネルのタイトル */
title: React.ReactNode
/**
Expand All @@ -28,6 +36,8 @@ type Props = PropsWithChildren<{
}> &
VariantProps<typeof informationPanel>

type Props = AbstractProps & Omit<BaseElementProps, keyof AbstractProps>

const OPEN_BUTTON_LABEL = '開く'
const CLOSE_BUTTON_LABEL = '閉じる'

Expand Down Expand Up @@ -101,11 +111,11 @@ export const informationPanel = tv({
],
})

export const InformationPanel: FC<Props & Omit<BaseElementProps, keyof Props>> = ({
export const InformationPanel: FC<Props> = ({
title,
titleTag,
type = 'info',
togglable = false,
Copy link
Member Author

Choose a reason for hiding this comment

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

比較にしか利用されていないため、初期値代入をする必要がありませんでした

togglable,
active: activeProps = true,
bold,
className,
Expand All @@ -118,51 +128,124 @@ export const InformationPanel: FC<Props & Omit<BaseElementProps, keyof Props>> =
const titleId = useId()
const contentId = useId()

const handleClickTrigger = useCallback(() => {
if (onClickTrigger) {
onClickTrigger(active)
} else {
setActive(!active)
}
}, [active, onClickTrigger])
Comment on lines -121 to -127
Copy link
Member Author

Choose a reason for hiding this comment

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

TogglableButton内に移動しています


useEffect(() => {
setActive(activeProps)
}, [activeProps])

const { wrapper, header, heading, togglableButton, content } = informationPanel({
type,
active,
bold,
})
const styles = useMemo(() => {
const withActive = informationPanel({
type,
active: true,
bold,
})
const withInactive = informationPanel({
type,
active: false,
bold,
})

const wrapperProps = { className }

return {
active: {
wrapper: withActive.wrapper(wrapperProps),
header: withActive.header(),
heading: withActive.heading(),
togglableButton: withActive.togglableButton(),
content: withActive.content(),
},
inactive: {
wrapper: withInactive.wrapper(wrapperProps),
header: withInactive.header(),
heading: withInactive.heading(),
togglableButton: withInactive.togglableButton(),
content: withInactive.content(),
},
}
}, [bold, type, className])
Comment on lines +135 to +165
Copy link
Member Author

Choose a reason for hiding this comment

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

再生成する必要がないよう、active, inactiveのパターン両方を事前に生成しています

Copy link
Member Author

Choose a reason for hiding this comment

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

またコンポーネントを分割する関係上、関数を渡すよりstringを渡すほうがmemo化が適切に動作する可能性が高いためこのようなロジックにしています


const currentStyles = styles[active ? 'active' : 'inactive']

return (
<Base {...props} overflow="hidden" as="section" className={wrapper({ className })}>
<Cluster align="center" justify="space-between" className={header()}>
<Base {...props} overflow="hidden" as="section" className={currentStyles.wrapper}>
<Cluster align="center" justify="space-between" className={currentStyles.header}>
{/* eslint-disable-next-line smarthr/a11y-heading-in-sectioning-content */}
<Heading type="blockTitle" tag={titleTag} id={titleId} className={heading()}>
<ResponseMessage type={type} iconGap={0.5}>
{title}
</ResponseMessage>
</Heading>
<MemoizedHeading tag={titleTag} id={titleId} className={currentStyles.heading} type={type}>
{title}
</MemoizedHeading>
Comment on lines +173 to +175
Copy link
Member Author

Choose a reason for hiding this comment

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

Headingは再生成が必要になるパターンが少ないことが予想されるため、丸ごとmemo化しています

{togglable && (
<Button
suffix={active ? <FaCaretUpIcon /> : <FaCaretDownIcon />}
size="s"
onClick={handleClickTrigger}
aria-expanded={togglable ? active : undefined}
aria-controls={contentId}
className={togglableButton()}
>
{active
? decorators?.closeButtonLabel?.(CLOSE_BUTTON_LABEL) || CLOSE_BUTTON_LABEL
: decorators?.openButtonLabel?.(OPEN_BUTTON_LABEL) || OPEN_BUTTON_LABEL}
</Button>
<TogglableButton
active={active}
onClickTrigger={onClickTrigger}
setActive={setActive}
contentId={contentId}
className={currentStyles.togglableButton}
decorators={decorators}
/>
)}
</Cluster>
<div id={contentId} aria-hidden={!active} className={content()}>
<div id={contentId} aria-hidden={!active} className={currentStyles.content}>
{children}
</div>
</Base>
)
}

const MemoizedHeading = React.memo<
Pick<Props, 'type'> & {
tag: Props['titleTag']
id: string
className: string
children: Props['title']
}
>(({ type, children, ...rect }) => (
<Heading {...rect} type="blockTitle">
<ResponseMessage type={type} iconGap={0.5}>
{children}
</ResponseMessage>
</Heading>
))

const TogglableButton: React.FC<
Pick<Props, 'onClickTrigger' | 'decorators'> & {
active: boolean
setActive: (flg: boolean) => void
contentId: string
className: string
}
> = ({ active, onClickTrigger, setActive, contentId, className, decorators }) => {
const handleClickTrigger = useCallback(() => {
if (onClickTrigger) {
onClickTrigger(active)
} else {
setActive(!active)
}
}, [active, onClickTrigger, setActive])

const decoratedTexts = useMemo(() => {
if (!decorators) {
return {
active: CLOSE_BUTTON_LABEL,
inactive: OPEN_BUTTON_LABEL,
}
}

return {
active: decorators.closeButtonLabel?.(CLOSE_BUTTON_LABEL) || CLOSE_BUTTON_LABEL,
inactive: decorators.openButtonLabel?.(OPEN_BUTTON_LABEL) || OPEN_BUTTON_LABEL,
}
}, [decorators])

return (
<Button
suffix={active ? <FaCaretUpIcon /> : <FaCaretDownIcon />}
size="s"
onClick={handleClickTrigger}
aria-expanded={active}
Copy link
Member Author

Choose a reason for hiding this comment

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

もともとはtogglableのようなロジックでundefinedが設定される可能性がありましたが、このボタンが表示されている場合、togglableがfalsyになる場合はありえないため、調整しています

aria-controls={contentId}
className={className}
>
{decoratedTexts[active ? 'active' : 'inactive']}
</Button>
)
}
Loading