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: Heading/LanguageSwitcher, Icon, InformationPanelの内部ロジックを整理する #5433

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

import React, { HTMLAttributes, ReactNode, useCallback, useMemo } from 'react'
import React, {
type FC,
type HTMLAttributes,
type KeyboardEvent,
type MouseEvent,
type ReactNode,
memo,
useMemo,
} from 'react'
import { VariantProps, tv } from 'tailwind-variants'

import { type DecoratorsType } from '../../../hooks/useDecorators'
import { type DecoratorsType, useDecorators } from '../../../hooks/useDecorators'
import { tabbable } from '../../../libs/tabbable'
import { Button } from '../../Button'
import { Dropdown, DropdownContent, DropdownTrigger } from '../../Dropdown'
Expand All @@ -17,19 +25,43 @@ export type Props = {
locale?: string
defaultLocale?: string
/** コンポーネント内の文言を変更するための関数を設定 */
decorators?: DecoratorsType<'triggerLabel' | 'checkIconAlt'>
decorators?: DecoratorsType<DecoratorKeyTypes>
/** 言語切替UIで言語を選択した時に発火するコールバック関数 */
onLanguageSelect?: (code: string) => void
} & VariantProps<typeof appLauncher>
} & VariantProps<typeof classNameGenerator>

type ElementProps = Omit<HTMLAttributes<HTMLElement>, keyof Props>

const TRIGGER_LABEL = 'Language'
const CHECK_ICON_ALT = '選択中'
const DECORATOR_DEFAULT_TEXTS = {
triggerLabel: 'Language',
checkIconAlt: '選択中',
} as const
type DecoratorKeyTypes = keyof typeof DECORATOR_DEFAULT_TEXTS

const ARROW_KEY_REGEX = /^Arrow(Up|Down|Left|Right)$/
const ARROW_UPS_REGEX = /^Arrow(Up|Left)$/

const appLauncher = tv({
const ON_KEY_DOWN_CONTENT = (e: KeyboardEvent<HTMLDivElement>) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

stateなどと依存関係がないため、定数化しました

Copy link
Collaborator

Choose a reason for hiding this comment

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

imo: 関数も大文字にするの見慣れない感じがしました。特に定数感を強めなくても handleKeyDownContentkeyDownContentHandler などでも十分に見えます。

Copy link
Member Author

Choose a reason for hiding this comment

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

あー、どうなんだろう。
一応定数であることを明示するメリットはあるかなーと思いますが...

Copy link
Collaborator

Choose a reason for hiding this comment

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

(個人的には)あまり見慣れないなぁと思いました。
JS でファイルグローバルにある関数はみんな定数としての関数なのか? という感じ。

Copy link
Member Author

Choose a reason for hiding this comment

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

難しいな。
でも関数が定数形式の名称になってると、実行するときとかに違和感あるのは 🦀 なので キャメルケースに変更します。
onかhandleかはどうしましょうね....
#5433 (comment)

Copy link
Contributor

Copy link
Member Author

Choose a reason for hiding this comment

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

onになりました。

https://kufuinc.slack.com/archives/CGC58MW01/p1741237708852539

まとめ

  • どちらかに明確なメリットはないため、厳格に決定する必要はなさそう
  • その上で、smarthr-uiはコンポーネントライブラリなので既に onXxxx系の属性が存在する
  • 最終的にonXxxx系属性に設定されることになるものはonXxxxという名前にしたほうがわかりやすい
    • 逆にonXxxx系属性に設定されない関数に関してはこのルールを適用することはない(しない方が良い場合が多い)
  • 上記から記述的に短いこともありonXxxxというフォーマットを採用します

if (!ARROW_KEY_REGEX.test(e.key)) {
return
}

e.preventDefault()

const buttons = tabbable(e.currentTarget)
const i = buttons.indexOf(e.target as HTMLElement)
let buttonAt = 0

if (ARROW_UPS_REGEX.test(e.key)) {
buttonAt = i - 1
} else if (i + 1 === buttons.length) {
buttonAt = i + 1
}

buttons.at(buttonAt)?.focus()
}

const classNameGenerator = tv({
slots: {
switchButton: [
'shr-border-none shr-font-normal shr-text-white shr-transition-transform shr-duration-100 shr-bg-transparent shr-px-0.25',
Expand Down Expand Up @@ -64,7 +96,7 @@ const appLauncher = tv({
},
})

export const LanguageSwitcher: React.FC<Props & ElementProps> = ({
export const LanguageSwitcher: FC<Props & ElementProps> = ({
narrow,
enableNew,
invert = enableNew,
Expand All @@ -75,26 +107,17 @@ export const LanguageSwitcher: React.FC<Props & ElementProps> = ({
onLanguageSelect,
...rest
}) => {
const locales = useMemo(() => Object.entries(localeMap), [localeMap])
const decoratedTexts = useMemo(() => {
if (!decorators) {
return {
triggerLabel: TRIGGER_LABEL,
checkIconAlt: CHECK_ICON_ALT,
}
}

return {
triggerLabel: decorators.triggerLabel?.(TRIGGER_LABEL) || TRIGGER_LABEL,
checkIconAlt: decorators.checkIconAlt?.(CHECK_ICON_ALT) || CHECK_ICON_ALT,
}
}, [decorators])
const currentLang = useMemo(
() => locale || defaultLocale || Object.keys(localeMap)[0],
[locale, defaultLocale, localeMap],
const { locales, defaultCurrentLang } = useMemo(
() => ({
locales: Object.entries(localeMap),
defaultCurrentLang: Object.keys(localeMap)[0],
}),
[localeMap],
)
const styles = useMemo(() => {
const { languageButton, languageItemsList, languageItem, switchButton } = appLauncher()
const decorated = useDecorators<DecoratorKeyTypes>(DECORATOR_DEFAULT_TEXTS, decorators)
const currentLang = locale || defaultLocale || defaultCurrentLang
const classNames = useMemo(() => {
const { languageButton, languageItemsList, languageItem, switchButton } = classNameGenerator()

return {
languageButton: languageButton(),
Expand All @@ -104,55 +127,35 @@ export const LanguageSwitcher: React.FC<Props & ElementProps> = ({
}
}, [enableNew, invert])

const handleLanguageSelect = useMemo(
const onClickLanguageSelect = useMemo(
() =>
onLanguageSelect
? (e: React.MouseEvent<HTMLButtonElement>) => {
? (e: MouseEvent<HTMLButtonElement>) => {
onLanguageSelect(e.currentTarget.value)
}
: undefined,
[onLanguageSelect],
)

const handleKeyDown = useCallback((e: React.KeyboardEvent<HTMLDivElement>) => {
if (!ARROW_KEY_REGEX.test(e.key)) {
return
}

e.preventDefault()

const buttons = tabbable(e.currentTarget)
const i = buttons.indexOf(e.target as HTMLElement)
let buttonAt = 0

if (ARROW_UPS_REGEX.test(e.key)) {
buttonAt = i - 1
} else if (i + 1 === buttons.length) {
buttonAt = i + 1
}

buttons.at(buttonAt)?.focus()
}, [])

return (
<Dropdown {...rest}>
<MemoizedDropdownTrigger
narrow={narrow}
invert={invert}
className={styles.switchButton}
label={decoratedTexts.triggerLabel}
className={classNames.switchButton}
label={decorated.triggerLabel}
/>
<DropdownContent onKeyDown={handleKeyDown} role="presentation">
<ul className={styles.languageItemsList}>
<DropdownContent role="presentation" onKeyDown={ON_KEY_DOWN_CONTENT}>
Copy link
Contributor

Choose a reason for hiding this comment

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

ここも同様の理由でキャメルケースに戻すと良いですかね?

<ul className={classNames.languageItemsList}>
{locales.map(([code, label]) => (
<LanguageListItemButton
key={code}
code={code}
className={styles.languageItem}
buttonStyle={styles.languageButton}
className={classNames.languageItem}
buttonStyle={classNames.languageButton}
current={currentLang === code}
handleLanguageSelect={handleLanguageSelect}
iconAlt={decoratedTexts.checkIconAlt}
onClick={onClickLanguageSelect}
iconAlt={decorated.checkIconAlt}
>
{label}
</LanguageListItemButton>
Expand All @@ -163,19 +166,19 @@ export const LanguageSwitcher: React.FC<Props & ElementProps> = ({
)
}

const LanguageListItemButton = React.memo<{
const LanguageListItemButton = memo<{
code: string
children: string
className: string
buttonStyle: string
current: boolean
iconAlt: ReactNode
handleLanguageSelect?: (e: React.MouseEvent<HTMLButtonElement>) => void
}>(({ code, children, buttonStyle, className, current, iconAlt, handleLanguageSelect }) => (
onClick?: (e: MouseEvent<HTMLButtonElement>) => void
}>(({ code, children, buttonStyle, className, current, iconAlt, onClick }) => (
<li key={code} className={className} aria-current={current} lang={code}>
<Button
value={code}
onClick={handleLanguageSelect}
onClick={onClick}
wide
prefix={current ? <FaCheckIcon color="MAIN" alt={iconAlt} /> : null}
className={buttonStyle}
Expand All @@ -185,7 +188,7 @@ const LanguageListItemButton = React.memo<{
</li>
))

const MemoizedDropdownTrigger = React.memo<
const MemoizedDropdownTrigger = memo<
Pick<Props, 'narrow' | 'invert'> & { className: string; label: ReactNode }
>(({ narrow, invert, className, label }) => (
<DropdownTrigger>
Expand Down
29 changes: 15 additions & 14 deletions packages/smarthr-ui/src/components/Icon/generateIcon.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import React, { ComponentProps, useMemo } from 'react'
import { IconType } from 'react-icons'
import React, { type ComponentProps, type ReactNode, memo, useMemo } from 'react'
import { tv } from 'tailwind-variants'

import { colors, fontSize, textColor } from '../../themes'
import { FontSizes } from '../../themes/createFontSize'
import { AbstractSize, CharRelativeSize } from '../../themes/createSpacing'
import { Gap } from '../../types'
import { VisuallyHiddenText } from '../VisuallyHiddenText'

import type { FontSizes } from '../../themes/createFontSize'
import type { AbstractSize, CharRelativeSize } from '../../themes/createSpacing'
import type { Gap } from '../../types'
import type { IconType } from 'react-icons'

/**
* literal union type に補完を効かせるためのハック
* https://github.com/microsoft/TypeScript/issues/29729
Expand Down Expand Up @@ -55,21 +56,21 @@ type ElementProps = Omit<ComponentProps<'svg'>, keyof IconProps>

type BaseComponentProps = {
/**アイコンの説明テキスト*/
alt?: React.ReactNode
alt?: ReactNode
/** アイコンと並べるテキスト */
text?: React.ReactNode
text?: ReactNode
/** アイコンと並べるテキストとの溝 */
iconGap?: CharRelativeSize | AbstractSize
/** `true` のとき、アイコンを右側に表示する */
right?: boolean
}
export type Props = Omit<IconProps & ElementProps, keyof BaseComponentProps> & BaseComponentProps

const icon = tv({
const iconClassNameGenerator = tv({
base: 'smarthr-ui-Icon group-[]/iconWrapper:shr-shrink-0 group-[]/iconWrapper:shr-translate-y-[0.125em] forced-colors:shr-fill-[CanvasText]',
})

const wrapper = tv({
const wrapperClassNameGenerator = tv({
base: ['smarthr-ui-Icon-withText shr-group/iconWrapper shr-inline-flex shr-items-baseline'],
variants: {
gap: {
Expand Down Expand Up @@ -112,7 +113,7 @@ const wrapper = tv({
})

export const generateIcon = (SvgIcon: IconType) => {
const Icon = React.memo<Props>(
const Icon = memo<Props>(
({
color,
className,
Expand Down Expand Up @@ -140,8 +141,8 @@ export const generateIcon = (SvgIcon: IconType) => {
return undefined
}, [ariaHidden, alt, ariaLabel, ariaLabelledby])

const iconStyle = useMemo(() => icon({ className }), [className])
const wrapperStyle = useMemo(() => wrapper({ gap: iconGap }), [iconGap])
const iconClassName = useMemo(() => iconClassNameGenerator({ className }), [className])
const wrapperClassName = useMemo(() => wrapperClassNameGenerator({ gap: iconGap }), [iconGap])

const replacedColor = useMemo(() => {
if (color && existsColor(color)) {
Expand Down Expand Up @@ -169,7 +170,7 @@ export const generateIcon = (SvgIcon: IconType) => {
width={iconSize}
height={iconSize}
color={replacedColor}
className={iconStyle}
className={iconClassName}
role={role}
aria-hidden={actualAriaHidden}
aria-label={ariaLabel}
Expand All @@ -181,7 +182,7 @@ export const generateIcon = (SvgIcon: IconType) => {

if (text) {
return (
<span className={wrapperStyle}>
<span className={wrapperClassName}>
{right && text}
{visuallyHiddenAlt}
{svgIcon}
Expand Down
Loading
Loading