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: ActionDialogの内部処理を整理する #5324

Merged
merged 11 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const ActionDialog: React.FC<Props & ElementProps> = ({
onClickClose,
onPressEscape = onClickClose,
responseMessage,
actionDisabled = false,
actionDisabled,
Copy link
Member Author

Choose a reason for hiding this comment

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

最終的にButtonのdisabledに設定されるだけだったため、初期化の意味が薄く、削除しました

closeDisabled,
subActionArea,
className,
Expand All @@ -37,17 +37,15 @@ export const ActionDialog: React.FC<Props & ElementProps> = ({
const titleId = useId()

const handleClickClose = useCallback(() => {
if (!props.isOpen) {
return
if (props.isOpen) {
onClickClose()
Copy link
Member Author

Choose a reason for hiding this comment

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

早期returnしていますが

  • 直後の処理の判定を逆転させているだけであり、理解が一手遅れる
  • !とreturn分の処理が余計にかかる

というデメリットのほうが大きそうだったため調整しています

}
onClickClose()
}, [onClickClose, props.isOpen])

const handleClickAction = useCallback(() => {
if (!props.isOpen) {
return
if (props.isOpen) {
onClickAction(onClickClose)
}
onClickAction(onClickClose)
}, [onClickAction, onClickClose, props.isOpen])

return createPortal(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const ActionDialogContent: React.FC<Props & ElementProps> = ({
actionText,
actionTheme,
onClickAction,
actionDisabled = false,
actionDisabled,
portalParent,
className = '',
decorators,
Expand All @@ -30,17 +30,15 @@ export const ActionDialogContent: React.FC<Props & ElementProps> = ({
const { createPortal } = useDialogPortal(portalParent)

const handleClickClose = useCallback(() => {
if (!active) {
return
if (active) {
onClickClose()
}
onClickClose()
}, [active, onClickClose])

const handleClickAction = useCallback(() => {
if (!active) {
return
if (active) {
onClickAction(onClickClose)
}
onClickAction(onClickClose)
}, [active, onClickAction, onClickClose])

const titleId = useId()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
'use client'

import React, { type FC, type PropsWithChildren, type ReactNode, useCallback } from 'react'
import React, { type FC, type PropsWithChildren, type ReactNode, useCallback, useMemo } from 'react'

import { Button } from '../../Button'
import { Cluster, Stack } from '../../Layout'
import { ResponseMessage } from '../../ResponseMessage'
import { Section } from '../../SectioningContent'
import { DialogBody, type Props as DialogBodyProps } from '../DialogBody'
import { DialogHeader, type Props as DialogHeaderProps } from '../DialogHeader'
import { dialogContentInner } from '../dialogInnerStyle'
Expand Down Expand Up @@ -40,6 +41,8 @@ export type ActionDialogContentInnerProps = BaseProps & {
}

const CLOSE_BUTTON_LABEL = 'キャンセル'
const ACTION_AREA_CLUSTER_GAP = { row: 0.5, column: 1 } as const

export const ActionDialogContentInner: FC<ActionDialogContentInnerProps> = ({
children,
title,
Expand All @@ -49,59 +52,166 @@ export const ActionDialogContentInner: FC<ActionDialogContentInnerProps> = ({
contentBgColor,
contentPadding,
actionText,
actionTheme = 'primary',
actionTheme,
onClickAction,
onClickClose,
responseMessage,
actionDisabled = false,
actionDisabled,
closeDisabled,
subActionArea,
decorators,
}) => {
const handleClickAction = useCallback(() => {
onClickAction(onClickClose)
}, [onClickAction, onClickClose])
const isRequestProcessing = responseMessage && responseMessage.status === 'processing'
const calcedResponseStatus = useMemo(() => {
Copy link
Member Author

Choose a reason for hiding this comment

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

responseMessageの判定が煩雑になっていたため、扱いやすい形に変換、memo化しています。
この処理は他のコンポーネントでもよくあるもののため、カスタムhookを別PRで作成予定です

if (!responseMessage) {
return {
isProcessing: false,
visibleMessage: false,
}
}

if (responseMessage.status === 'processing') {
return {
isProcessing: true,
visibleMessage: false,
}
}

const { wrapper, actionArea, buttonArea, message } = dialogContentInner()
return {
isProcessing: false,
visibleMessage: true,
// HINT: statusがprocessingではない === success or errorであることが確定する
// success or error の場合、text属性も必ず存在する
status: responseMessage.status as 'success' | 'error',
message: (responseMessage as { text: string }).text,
}
}, [responseMessage])

const styles = useMemo(() => {
const { wrapper, actionArea, buttonArea, message } = dialogContentInner()

return {
wrapper: wrapper(),
actionArea: actionArea(),
buttonArea: buttonArea(),
message: message(),
}
}, [])

return (
// eslint-disable-next-line smarthr/best-practice-for-layouts, smarthr/a11y-heading-in-sectioning-content
<Stack gap={0} as="section" className={wrapper()}>
// eslint-disable-next-line smarthr/a11y-heading-in-sectioning-content
<Section className={styles.wrapper}>
<DialogHeader title={title} subtitle={subtitle} titleTag={titleTag} titleId={titleId} />
<DialogBody contentPadding={contentPadding} contentBgColor={contentBgColor}>
{children}
</DialogBody>
<Stack gap={0.5} className={actionArea()}>
<Stack gap={0.5} className={styles.actionArea}>
<Cluster justify="space-between">
{subActionArea}
<Cluster gap={{ row: 0.5, column: 1 }} className={buttonArea()}>
<Button
onClick={onClickClose}
disabled={closeDisabled || isRequestProcessing}
className="smarthr-ui-Dialog-closeButton"
>
{decorators?.closeButtonLabel?.(CLOSE_BUTTON_LABEL) || CLOSE_BUTTON_LABEL}
</Button>
<Button
variant={actionTheme}
onClick={handleClickAction}
disabled={actionDisabled}
loading={isRequestProcessing}
className="smarthr-ui-Dialog-actionButton"
>
{actionText}
</Button>
</Cluster>
<ActionAreaCluster
onClickClose={onClickClose}
onClickAction={onClickAction}
closeDisabled={closeDisabled}
actionDisabled={actionDisabled}
loading={calcedResponseStatus.isProcessing}
actionTheme={actionTheme}
decorators={decorators}
actionText={actionText}
className={styles.buttonArea}
/>
</Cluster>
{(responseMessage?.status === 'success' || responseMessage?.status === 'error') && (
<div className={message()}>
<ResponseMessage type={responseMessage.status} role="alert">
{responseMessage.text}
{calcedResponseStatus.visibleMessage && (
<div className={styles.message}>
<ResponseMessage type={calcedResponseStatus.status} role="alert">
{calcedResponseStatus.message}
</ResponseMessage>
</div>
)}
</Stack>
</Stack>
</Section>
)
}

const ActionAreaCluster = React.memo<
Pick<
ActionDialogContentInnerProps,
| 'onClickClose'
| 'onClickAction'
| 'closeDisabled'
| 'actionDisabled'
| 'actionTheme'
| 'decorators'
| 'actionText'
> & { loading: boolean; className: string }
>(
({
onClickClose,
onClickAction,
closeDisabled,
actionDisabled,
loading,
actionTheme,
decorators,
actionText,
className,
}) => {
const handleClickAction = useCallback(() => {
onClickAction(onClickClose)
}, [onClickAction, onClickClose])

return (
<Cluster gap={ACTION_AREA_CLUSTER_GAP} className={className}>
<CloseButton
onClick={onClickClose}
disabled={closeDisabled || loading}
decorators={decorators}
/>
<ActionButton
variant={actionTheme}
disabled={actionDisabled}
loading={loading}
onClick={handleClickAction}
>
{actionText}
</ActionButton>
</Cluster>
)
},
)

const ActionButton = React.memo<
PropsWithChildren<{
variant: ActionDialogContentInnerProps['actionTheme']
disabled: ActionDialogContentInnerProps['actionDisabled']
loading: boolean
onClick: () => void
}>
>(({ variant = 'primary', disabled, loading, onClick, children }) => (
<Button
type="submit"
variant={variant}
disabled={disabled}
loading={loading}
onClick={onClick}
className="smarthr-ui-Dialog-actionButton"
>
{children}
</Button>
))

const CloseButton = React.memo<
Pick<ActionDialogContentInnerProps, 'decorators'> & {
onClick: ActionDialogContentInnerProps['onClickClose']
disabled: boolean
}
>(({ onClick, disabled, decorators }) => {
const children = useMemo(
() => decorators?.closeButtonLabel?.(CLOSE_BUTTON_LABEL) || CLOSE_BUTTON_LABEL,
[decorators],
)

return (
<Button onClick={onClick} disabled={disabled} className="smarthr-ui-Dialog-closeButton">
{children}
</Button>
)
})
Original file line number Diff line number Diff line change
Expand Up @@ -21,29 +21,20 @@ export const ActionDialogWithTrigger: React.FC<
const open = useCallback(() => setIsOpen(true), [])
const close = useCallback(() => setIsOpen(false), [])

const onClickOpen = useCallback(() => {
if (onClickTrigger) {
return onClickTrigger(open)
}

open()
}, [onClickTrigger, open])

const actualOnClickClose = useCallback(() => {
if (onClickClose) {
return onClickClose(close)
}

close()
}, [onClickClose, close])
const onClickOpen = useMemo(
() => (onClickTrigger ? () => onClickTrigger(open) : open),
[onClickTrigger, open],
)
Comment on lines +24 to +27
Copy link
Member Author

Choose a reason for hiding this comment

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

useCallbackではなくuseMemoにしています。
こうすることでどの場合でもonClickOpen実行時に行っているif判定を事前に行えます


const actualOnPressEscape = useCallback(() => {
if (onPressEscape) {
return onPressEscape(close)
}
const actualOnClickClose = useMemo(
() => (onClickClose ? () => onClickClose(close) : close),
[onClickClose, close],
)

close()
}, [onPressEscape, close])
const actualOnPressEscape = useMemo(
() => (onPressEscape ? () => onPressEscape(close) : close),
[onPressEscape, close],
)

const actualTrigger = useMemo(
() =>
Expand Down
Loading