chore: AppNavi, BottomFixedArea, MultiComboboxの内部処理を整理する #5441
Conversation
AtsushiM
changed the title
commit: |
| const Description = memo<PropsWithChildren>( | ||
| ({ children }) => | ||
| children && <p className="smarthr-ui-BottomFixedArea-description">{children}</p>, | ||
| ) |
There was a problem hiding this comment.
descriptionが変化する可能性は低いため、memo化しています
yagimushi
force-pushed
the
fix-format-class-name-generator-12
branch
from
71f25bf to
1edd4ac
Compare
AtsushiM
requested review from
yt-ymmt and
yuzuru-akiyama
and removed request for
a team
|
nits）BottomFixedAreaの型情報、まとめちゃっても良いかも？と思いました。 export type ButtonElement =
| FunctionComponentElement<ComponentProps<typeof Button>>
| FunctionComponentElement<ComponentProps<typeof AnchorButton>>
export type Primary = ButtonElement
export type Secondary = ButtonElement |
|
@AtsushiM yuzuru sanがレビュアーになっておりますが、terraformの設定ミスの名残です:pray: |
ああ、全く一緒なんですね |
…format-class-name-generator-12
| if (isComposing) { | ||
| return | ||
| } |
There was a problem hiding this comment.
はい、駄目です。
以前別PRで話した内容と同じ修正ですね。
#5433 (comment)
There was a problem hiding this comment.
初見では意図のわかりにくい変更が多かったため、レビューコメントでも問題ので補足入れてもらえると嬉しいです!
| const actualOnDelete = useMemo(() => { | ||
| const handlers: Array<(item: ComboBoxItem<T>) => void> = [] | ||
|
|
||
| if (onDelete) { | ||
| handlers.push((item: ComboBoxItem<T>) => onDelete(item)) | ||
| } | ||
| if (onChangeSelected) { | ||
| handlers.push((item: ComboBoxItem<T>) => | ||
| onChangeSelected( | ||
| selectedItems.filter((selected) => !areComboBoxItemsEqual(selected, item)), | ||
| ), | ||
| ) | ||
| } | ||
|
|
||
| if (handlers.length === 0) { | ||
| return NOOP | ||
| } |
There was a problem hiding this comment.
この関数は何もする必要がないパターンが存在しており、それのチェックを追加しています。
以前の場合、何もする必要がない場合でもrequestAnimationFrameの呼び出しなどは行われていました。
#5433 (comment) と同じ理屈です。
そのうえで、この関数は他の関数内からの呼び出しが存在しており、かつ別ファイルで関数が常にあることを期待している状態だったため、NOOPを返すようにしています。
定数のNOOPを返すことで、他のhookのmemo化も適切に処理されます。
まとめて修正できるならundefinedにしたいところですが一旦ここまでで止めています。
関連URL
概要
変更内容
確認方法