ツール系のStateをJotaiに置き換え #264 #265
Conversation
- Jotaiライブラリのインストール - toolやtoolに関するアクションをpropから除外し、Jotaiライブラリが管理するように修正 - front/src/store/event/venueedit/tool.tsでStateやdispatchを管理し、useSubToolSelectionフック経由で各コンポーネントから状態を取得&操作できるように - isTextTool(selectedTool)のような冗長な関数呼び出しではなく、変数としてそのまま使えるように(isDrawingToolやisPixelToolも同様)
- 余計な関数を除去
There was a problem hiding this comment.
Pull Request Overview
This PR refactors tool-related state management in the venue editor from React's useState to Jotai library, eliminating the need to pass selectedTool and its manipulation functions through props.
- Introduces centralized state management for tool selection using Jotai atoms
- Removes prop drilling by providing tool state and actions through a custom hook
- Simplifies component interfaces by removing tool-related props
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| front/package.json | Adds Jotai library dependency |
| front/src/store/event/venueedit/tool.ts | Implements Jotai atoms for tool state management with action-based updates |
| front/src/hooks/event/venueedit/useSubToolSelection.ts | Refactors hook to use Jotai atoms and adds memoized tool type checks |
| front/src/hooks/event/venueedit/useCanvasDraw.ts | Removes selectedTool prop dependency |
| front/src/hooks/event/venueedit/tool/useToolSelect.ts | Gets selectedTool from custom hook instead of props |
| front/src/components/templates/event/venueedit/*.tsx | Removes tool-related props from template components |
| front/src/components/organisms/event/venueedit/tools/ToolButtonGroup.tsx | Uses hook for tool state instead of props |
| front/src/components/organisms/event/venueedit/palette/*.tsx | Removes tool-related props and uses hook for state access |
Files not reviewed (1)
- front/package-lock.json: Language not supported
| | { type: 'SET_DRAW_TOOL'; toolKey: EditorToolPairKey }; | ||
|
|
||
| // 状態を保持するプライベートなベースatom | ||
| const _selectedToolAtom = atom<VenueEditTool>('ピクセル塗りつぶし'); |
There was a problem hiding this comment.
The default tool value 'ピクセル塗りつぶし' is a magic string. Consider defining this as a constant to improve maintainability and prevent typos.
| const _selectedToolAtom = atom<VenueEditTool>('ピクセル塗りつぶし'); | |
| const _selectedToolAtom = atom<VenueEditTool>(DEFAULT_TOOL); |
| // isDrawingToolはselectedToolに依存し、その結果をメモ化する | ||
| const isDrawingTool = useMemo(() => _isDrawingTool(selectedTool), [selectedTool]); |
There was a problem hiding this comment.
[nitpick] The useMemo for isDrawingTool may be unnecessary overhead. Simple boolean checks like _isDrawingTool(selectedTool) are typically fast enough that memoization adds more complexity than benefit.
| // isDrawingToolはselectedToolに依存し、その結果をメモ化する | |
| const isDrawingTool = useMemo(() => _isDrawingTool(selectedTool), [selectedTool]); | |
| // isDrawingToolはselectedToolに依存し、その結果を直接計算する | |
| const isDrawingTool = _isDrawingTool(selectedTool); |
| // isPixelToolはselectedToolに依存し、その結果をメモ化する | ||
| const isPixelTool = useMemo(() => (PIXEL_TOOLS as readonly string[]).includes(selectedTool), [selectedTool]); |
There was a problem hiding this comment.
[nitpick] The useMemo for isPixelTool may be unnecessary overhead. Simple array includes operations are typically fast enough that memoization adds more complexity than benefit.
| // isPixelToolはselectedToolに依存し、その結果をメモ化する | |
| const isPixelTool = useMemo(() => (PIXEL_TOOLS as readonly string[]).includes(selectedTool), [selectedTool]); | |
| // isPixelToolはselectedToolに依存する | |
| const isPixelTool = (PIXEL_TOOLS as readonly string[]).includes(selectedTool); |
| // isTextToolはselectedToolに依存し、その結果をメモ化する | ||
| const isTextTool = useMemo(() => (TEXT_TOOLS as readonly string[]).includes(selectedTool), [selectedTool]); |
There was a problem hiding this comment.
[nitpick] The useMemo for isTextTool may be unnecessary overhead. Simple array includes operations are typically fast enough that memoization adds more complexity than benefit.
| // isTextToolはselectedToolに依存し、その結果をメモ化する | |
| const isTextTool = useMemo(() => (TEXT_TOOLS as readonly string[]).includes(selectedTool), [selectedTool]); | |
| // isTextToolはselectedToolに依存する | |
| const isTextTool = (TEXT_TOOLS as readonly string[]).includes(selectedTool); |
- ハードコーディング部分の修正
|
セルフマージしました |
間取りエディタにおいて、tool関連のStateをJotaiライブラリ管理に置き換えました。
これにより、selectedToolそのものや、それを操作する関数をPropsで渡す必要がなくなりました。
エディタ上で差分を確認すると次のようになっています。
コンポーネントから操作したい場合は次のようにカスタムフック経由でstateや関数を取得できます。
例：front/src/components/organisms/event/venueedit/tools/ToolButtonGroup.tsx
Propsがなくても状態を管理することができます。
以下、コミットメッセージのコピーです。