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: ComboBoxのuseOptionsのロジックを整理する #5332

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

AtsushiM
Copy link
Member

関連URL

概要

変更内容

確認方法

@yagimushi yagimushi force-pushed the chore-refactoring-ComboBox-useOptions branch from 802117a to 0df395e Compare January 25, 2025 01:41
Copy link

pkg-pr-new bot commented Jan 25, 2025

Open in Stackblitz

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

commit: ffb3ad7

Comment on lines +29 to +32
const isSelected = useCallback(
(item: ComboBoxItem<T>) => selected !== null && selected.label === item.label,
[selected],
)
Copy link
Member Author

Choose a reason for hiding this comment

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

isSelectedはoptionの数分実行されることになるため、配列チェックなどが多数実行されることになり、SingleComboBoxとMultiComboBoxで共通化するメリットが少なかったため、hookを分割しました

Comment on lines -38 to -41
const getOptionId = useCallback(
(optionIndex: number) => `${optionIdPrefix}-${optionIndex}`,
[optionIdPrefix],
)
Copy link
Member Author

Choose a reason for hiding this comment

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

メソッド化する意味が薄かったため削除しています

Comment on lines +70 to +80
const addingOption: ComboBoxOption<T> | null = useMemo(
() =>
creatable && inputValue && items.every((item) => item.label !== inputValue)
? {
id: newItemId,
isNew: true,
selected: false,
item: { label: inputValue, value: inputValue },
}
: null,
[inputValue, items, creatable, newItemId],
Copy link
Member Author

Choose a reason for hiding this comment

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

creatableがtrueになる可能性はかなり低く、nullになる可能性が高くなります。
別途メモ化することで他のmemoが更新される可能性が下がります

return allOptions
}
return allOptions.filter(({ item: { label } }) => {
if (!inputValue) return true
Copy link
Member Author

Choose a reason for hiding this comment

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

filter内で処理する意味がないため、isFilteringDisabledと同時にチェックするようにしています

@AtsushiM AtsushiM marked this pull request as ready for review January 25, 2025 02:01
@AtsushiM AtsushiM requested a review from a team as a code owner January 25, 2025 02:01
@AtsushiM AtsushiM requested review from moshisora and hiroki0525 and removed request for a team January 25, 2025 02:01
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.

1 participant