chore: hooksのロジックを整理する #5361
Open
chore: hooksのロジックを整理する #5361
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e-refactoring-hooks
commit: |
AtsushiM
commented
Feb 13, 2025
Comment on lines
+8
to
+24
| useEffect(() => { | ||
| const handleClick = (e: MouseEvent) => { | ||
| if (innerRefs.some((target) => isEventIncludedParent(e, target.current))) { | ||
| innerCallback(e) | ||
|
|
||
| return | ||
| } | ||
|
|
||
| outerCallback(e) | ||
| }, | ||
| // spread innerRefs to compare deps one by one | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| [...innerRefs, innerCallback, outerCallback], | ||
| ) | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| window.addEventListener('click', handleClick) | ||
|
|
||
| return () => { | ||
| window.removeEventListener('click', handleClick) | ||
| } | ||
| }, [handleClick]) | ||
| }, [innerRefs, innerCallback, outerCallback]) |
There was a problem hiding this comment.
handleClickはuseEffectの中でしか利用されていないため、ひとまとめにしています。
依存関係の配列も展開は微妙なのでmemo化したものが渡されてくるように修正しています
AtsushiM
commented
Feb 13, 2025
Comment on lines
+28
to
+32
| if (!parent) return false | ||
|
|
||
| const path = e.composedPath() | ||
| if (path.length === 0 || !parent) return false | ||
|
|
||
| if (path.length === 0) return false |
There was a problem hiding this comment.
pathの生成が不要なパターンに対するチェックを最適化しています
AtsushiM
commented
Feb 13, 2025
Comment on lines
8
to
18
| useEffect(() => { | ||
| const handleKeyPress = (e: KeyboardEvent) => { | ||
| if (ESCAPE_KEY_REGEX.test(e.key)) { | ||
| cb() | ||
| } | ||
| }, | ||
| [cb], | ||
| ) | ||
| useEffect(() => { | ||
| } | ||
|
|
||
| document.addEventListener('keydown', handleKeyPress) | ||
|
|
||
| return () => document.removeEventListener('keydown', handleKeyPress) | ||
| }, [handleKeyPress]) | ||
| }, [cb]) |
There was a problem hiding this comment.
handleKeyPressはuseEffect中でしか利用されていないため、ひとまとめにしています
AtsushiM
commented
Feb 13, 2025
Comment on lines
-7
to
+12
| const handleOuterClick = useCallback( | ||
| (e: MouseEvent) => { | ||
| if (targets.some((target) => isEventIncludedParent(e, target.current))) { | ||
| return | ||
| useEffect(() => { | ||
| const handleOuterClick = (e: MouseEvent) => { | ||
| if (targets.every((target) => isEventExcludedParent(e, target.current))) { | ||
| callback(e) | ||
| } | ||
| callback(e) | ||
| }, | ||
| // spread targets to compare deps one by one | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| [...targets, callback], | ||
| ) | ||
| } |
There was a problem hiding this comment.
早期returnが直後の処理の逆転条件でしかないため、理解がワンテンポ遅れることになり、デメリットしかない状態になっています。
条件を逆転させることでわかりやすくしています
AtsushiM
commented
Feb 13, 2025
Comment on lines
+29
to
+36
| const calculatedSeqs = useMemo(() => { | ||
| const parentSeqs = parent.seqs.concat(currentSeq) | ||
|
|
||
| return { | ||
| parentSeqs, | ||
| portalChildOf: parentSeqs.join(','), | ||
| } | ||
| }, [currentSeq, parent.seqs]) |
There was a problem hiding this comment.
このファイルがやっていることはややこしいのですが、ざっくりいうと以下のとおりです
- アプリ全体でのPortalにシーケンスnoを振ります
- Portalを開く毎に紐づくnoをdata属性に追加し、現在開かれているportalが分かる状態にします
- 上記を利用して、特定のPortalがなにかのPortalの子になっているか？をチェックできるようにします
このチェックのため、data属性を利用していますが、data属性は基本stringしか突っ込めないので、joinでつなげるようにしています
AtsushiM
commented
Feb 13, 2025
Comment on lines
-90
to
+102
| const childOf = element.dataset?.portalChildOf || '' | ||
| const includesSeq = childOf.split(',').includes(String(parentPortalSeq)) | ||
| return includesSeq || _isChildPortal(element.parentElement, parentPortalSeq) | ||
|
|
||
| let includesSeq = false | ||
| const childOf = element.dataset?.portalChildOf | ||
|
|
||
| if (childOf) { | ||
| includesSeq = seqRegex.test(childOf) | ||
| } |
There was a problem hiding this comment.
これまでのロジックでは ,つなぎの数値をsplitして比較していましたが、シーケンス番号は数値を,でつなげたものであるため、正規表現でsplitしなくてもチェック可能にしました。
…e-refactoring-hooks
AtsushiM
requested review from
moshisora and
yuzuru-akiyama
and removed request for
a team
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
関連URL
概要
変更内容
確認方法