Skip to content

Conversation

@matamatanot
Copy link
Contributor

@matamatanot matamatanot commented Sep 3, 2025

関連URL

None

概要

Deprecatedなitems propsから置き換えようとしたらうまく動作しないケースが有ることに気が付きました。
以下のように条件分岐でDefinitionListItemを複数セットしたいケースがありますがこの場合にうまく動作しません。

			<DefinitionListItem term="入社日">
				{enteredDate}
			</DefinitionListItem>
			<DefinitionListItem term="退職日">
				{resignedDate}
			</DefinitionListItem>
			{isActive && (
				<>
					<DefinitionListItem term="加入者口座コード(21桁)">
						{hoge}
					</DefinitionListItem>
					<DefinitionListItem term="部店コード">
						{fuga}
					</DefinitionListItem>
				</>
			)}

変更内容

Fragmentか確認してchildrenを取り出して今までの処理を通すようにしました

備考

以下ならこのままでも動きます

			<DefinitionListItem term="入社日">
				{enteredDate}
			</DefinitionListItem>
			<DefinitionListItem term="退職日">
				{resignedDate}
			</DefinitionListItem>
			{isActive && (
				<DefinitionListItem term="口座コード">
					{hoge}
				</DefinitionListItem>
			)}
			{isActive && (
				<DefinitionListItem term="部店コード">
					{fuga}
				</DefinitionListItem>
			)}

@matamatanot matamatanot requested a review from a team as a code owner September 3, 2025 14:51
@matamatanot matamatanot requested review from AtsushiM and uknmr and removed request for a team September 3, 2025 14:51
@pkg-pr-new
Copy link

pkg-pr-new bot commented Sep 3, 2025

Open in StackBlitz

npm i https://pkg.pr.new/kufu/smarthr-ui@5839

commit: 1b9186e

@matamatanot matamatanot force-pushed the fix/definition-list-fragment-handling branch from 87a86e4 to 1b9186e Compare September 3, 2025 15:54
Comment on lines 59 to 73
{Children.toArray(children)
.flatMap((child) => {
if (isValidElement(child) && child.type === Fragment) {
return Children.toArray(child.props.children)
}
return child
})
.map((child) =>
isValidElement(child)
? cloneElement(child as ReactElement, {
maxColumns,
termStyleType,
})
: child,
)}
Copy link
Member

Choose a reason for hiding this comment

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

flatMap -> mapで配列を2回転している & 内容でisValidElementのチェックが2重でかかっている & Fragmentが複数ネストしている場合に対応できていないので以下の感じはどうっすかね?
(動作確認自体は未対応ですが...)

const childrenToItems = (children: ReactNode) => (
  Children.toArray(children)
    .reduce((prev, child) => {
      if (isValidElement(child)) {
        if (child.type === Fragment) {
          prev = prev.concat(childrenToItems(child.props.children))
        }

        prev.push(cloneElement(child as ReactElement, {
          maxColumns,
          termStyleType,
        }))
      } else {
        prev.push(child)
      }

      return prev
    }, [] as ReactElement)
)

...

{childrenToItems(children)}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AtsushiM
2回転とisValidElementの二重チェックを避けつつもう少し厳格にしました!
e1110ef

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants