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: CheckBoxの内部処理を最適化する #5340

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
94 changes: 45 additions & 49 deletions packages/smarthr-ui/src/components/CheckBox/CheckBox.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
'use client'

import React, {
ChangeEventHandler,
ComponentPropsWithRef,
PropsWithChildren,
forwardRef,
useCallback,
useEffect,
useId,
useImperativeHandle,
Expand Down Expand Up @@ -59,48 +57,29 @@ const checkbox = tv({
'shr-relative shr-box-border shr-inline-block shr-h-[theme(fontSize.base)] shr-w-[theme(fontSize.base)] shr-shrink-0 shr-translate-y-[0.125em] shr-leading-none',
label: [
'smarthr-ui-CheckBox-label shr-ms-0.5 shr-cursor-pointer shr-text-base shr-leading-tight',
'[[data-disabled=true]>&]:shr-pointer-events-none [[data-disabled=true]>&]:shr-cursor-not-allowed [[data-disabled=true]>&]:shr-text-disabled',
Copy link
Member Author

Choose a reason for hiding this comment

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

動的にclassを変更するのではなく、親要素の data-disabled を確認、それに応じてstyleを変更するようにしました。
これにより、memo化の効率が良くなります

Copy link
Contributor

Choose a reason for hiding this comment

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

Q: data-disabled より has-[input:disabled] のほうがsmarthr-ui内ではよく使われているかなと思ったんですが、data属性の方を選んだ理由ってありますか?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Qs-F
これややこしいんですが、labelはinputを内包していないんですよ。
構造的には

span[data-disabled]
  input
  label

となっています。

],
},
variants: {
disabled: {
true: {
label: 'shr-pointer-events-none shr-cursor-not-allowed shr-text-disabled',
},
},
},
})

export const CheckBox = forwardRef<HTMLInputElement, Props>(
({ checked, mixed = false, error, onChange, className, children, ...props }, ref) => {
const {
wrapperStyle,
innerWrapperStyle,
boxStyle,
inputStyle,
iconWrapStyle,
iconStyle,
labelStyle,
} = useMemo(() => {
({ checked, mixed, error, className, children, disabled, ...props }, ref) => {
const styles = useMemo(() => {
const { wrapper, innerWrapper, box, input, iconWrap, icon, label } = checkbox()

return {
wrapperStyle: wrapper({ className }),
innerWrapperStyle: innerWrapper(),
boxStyle: box(),
inputStyle: input(),
iconWrapStyle: iconWrap(),
iconStyle: icon(),
labelStyle: label({ disabled: props.disabled }),
wrapper: wrapper({ className }),
innerWrapper: innerWrapper(),
box: box(),
input: input(),
iconWrap: iconWrap(),
icon: icon(),
label: label(),
}
}, [className, props.disabled])

const handleChange = useCallback<ChangeEventHandler<HTMLInputElement>>(
(e) => {
if (onChange) onChange(e)
},
[onChange],
)
}, [className])

const inputRef = useRef<HTMLInputElement>(null)

useImperativeHandle<HTMLInputElement | null, HTMLInputElement | null>(
ref,
() => inputRef.current,
Expand All @@ -116,31 +95,48 @@ export const CheckBox = forwardRef<HTMLInputElement, Props>(
const checkBoxId = props.id || defaultId

return (
<span className={wrapperStyle}>
<span className={innerWrapperStyle}>
<span data-disabled={disabled?.toString()} className={styles.wrapper}>
<span className={styles.innerWrapper}>
<input
{...props}
data-smarthr-ui-input="true"
ref={inputRef}
type="checkbox"
id={checkBoxId}
checked={checked}
onChange={handleChange}
className={inputStyle}
ref={inputRef}
disabled={disabled}
aria-invalid={error || undefined}
className={styles.input}
data-smarthr-ui-input="true"
/>
<span className={boxStyle} aria-hidden="true" />
<span className={iconWrapStyle}>
{mixed ? <FaMinusIcon className={iconStyle} /> : <FaCheckIcon className={iconStyle} />}
</span>
<AriaHiddenBox className={styles.box} />
<CheckIconArea mixed={mixed} className={styles.iconWrap} iconStyle={styles.icon} />
</span>

{children && (
<label className={labelStyle} htmlFor={checkBoxId}>
{children}
</label>
)}
<LabeledChildren htmlFor={checkBoxId} className={styles.label}>
{children}
</LabeledChildren>
</span>
)
},
)

const AriaHiddenBox = React.memo<{ className: string }>(({ className }) => (
<span className={className} aria-hidden="true" />
))

const CheckIconArea = React.memo<Pick<Props, 'mixed'> & { className: string; iconStyle: string }>(
({ mixed, className, iconStyle }) => (
<span className={className}>
{mixed ? <FaMinusIcon className={iconStyle} /> : <FaCheckIcon className={iconStyle} />}
</span>
),
)

const LabeledChildren = React.memo<PropsWithChildren<{ className: string; htmlFor: string }>>(
({ children, htmlFor, className }) =>
children && (
<label htmlFor={htmlFor} className={className}>
{children}
</label>
),
)
Copy link
Member Author

Choose a reason for hiding this comment

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

これらの箇所はほぼ書き換わる可能性がないため、memo化しています。
LabeledChildrenはこのコンポーネント自体のchildrenを受け取っていますが、CheckBoxの特性上、ReactNodeでわたってくる可能性はひくく、ほとんどがstringとなるため、効率は良いはずです

1 change: 1 addition & 0 deletions packages/smarthr-ui/src/components/Chip/Chip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ const chip = tv({

export const Chip: FC<Props> = ({ size, disabled, className, ...props }) => {
const styles = useMemo(() => chip({ size, disabled, className }), [size, disabled, className])

return <span {...props} className={styles} />
}
Loading