[fix]タブ推移問題の修正#1986
Merged
batcho0428 merged 7 commits intogm3/developfrom Apr 26, 2026
Merged
Conversation
Contributor
|
今回のは調査からなので |
Collaborator
|
@riririn180904 |
There was a problem hiding this comment.
Pull request overview
このPRは、電力申請フォームでタブ切り替え時に入力内容がリセットされてしまう問題（#1973）を修正するものだよ〜✨
コピー&ペースト作業中にタブを切り替えると、フォームが再マウントされて入力中のデータが消えちゃうっていうユーザビリティ的にエグい問題があったんだよね😫 これを条件描画からCSS制御に変更することで、フォームのマウント状態を維持したまま表示/非表示を切り替えられるようにしたよ💪
Changes:
- フォームの表示制御を条件描画（
{showForm && ...}）からCSS制御（display: none）に変更して、タブ切り替え時のアンマウントを防止 - フォーム初期値の生成をuseMemoでラップして、不要な再生成を抑制
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| user/src/components/Applications/Power/hooks/usePowerApplication.ts | フォーム初期値の生成をuseMemoでメモ化し、hasExistingとdevicesが変更された時のみ再生成するように最適化 |
| user/src/components/Applications/Power/components/PowerFormView.tsx | 条件描画からstyleプロパティでのdisplay制御に変更し、フォームのアンマウントを防止 |
YosukeIida
force-pushed
the
fix/riririn/1973-Tab-transition
branch
from
f1219ca to
00228f0
Compare
batcho0428
requested review from
KokiWakatsuki
and removed request for
batcho0428 and
hikahana
…73-Tab-transition # Conflicts: # user/src/components/Applications/CookingProcessOrder/CookingProcessOrder.tsx # user/src/components/Applications/CookingProcessOrder/hooks.ts # user/src/components/Applications/FoodProduct/FoodProduct.tsx # user/src/components/Applications/FoodProduct/hooks.ts # user/src/components/Applications/Group/Group.tsx # user/src/components/Applications/Group/GroupForm/hooks.ts # user/src/components/Applications/Group/hooks.ts # user/src/components/Applications/PublicRelations/PublicRelations.tsx # user/src/components/Applications/PublicRelations/PublicRelationsForm/PublicRelationsForm.tsx # user/src/components/Applications/PublicRelations/hooks.ts # user/src/components/Applications/StageOptions/StageOptionForm/hooks.ts # user/src/components/Applications/StageOptions/StageOptions.tsx # user/src/components/Applications/StageOptions/hooks.ts # user/src/components/Applications/VenueMap/VenueMapForm/VenueMapForm.tsx # user/src/components/Applications/VenueMap/hooks.ts # user/src/components/Applications/ViceRepresentative/ViceRepresentative.tsx # user/src/components/Applications/ViceRepresentative/hook.ts # user/src/pages/home/index.tsx
📝 WalkthroughWalkthroughThis PR refactors form state management across multiple application components to prevent unintended form resets during tab transitions. Key changes include: (1) conditional API endpoint construction when groupId is undefined, (2) tri-state Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
user/src/components/Applications/Stage/hooks/useStageForm.ts (1)
80-103:⚠️ Potential issue | 🟠 MajorAdd
shouldDirty: trueandshouldTouch: truetosetValuecall to prevent reset effect from overwriting user edits.The
isDirtyguard in the useEffect (line 92) cannot prevent the reset whensetValueis called without theshouldDirtyoption. According to react-hook-form documentation,setValuedoes not updateformState.isDirtyby default. WithoutshouldDirty: true, user edits made viaupdateFieldremain invisible to the guard, allowing the reset effect to overwrite them when external data changes.Proposed fix
const updateField = (field: FormField, value: string) => { - setValue(field, value, { shouldValidate: true }); + setValue(field, value, { + shouldValidate: true, + shouldDirty: true, + shouldTouch: true, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/components/Applications/Stage/hooks/useStageForm.ts` around lines 80 - 103, The reset effect in useStageForm can overwrite user edits because updateField calls setValue without updating formState.isDirty/isTouched; modify the updateField function so its setValue call includes shouldDirty: true and shouldTouch: true (in addition to shouldValidate: true) to ensure user edits flip isDirty/isTouched and prevent the useEffect reset from running over in-progress edits (look for updateField, setValue, and the useEffect that calls reset).user/src/components/Applications/FoodProduct/FoodProduct.tsx (1)
49-65:⚠️ Potential issue | 🟡 MinorUse i18n for the "Loading..." label and consider consolidating the two loading branches.
Line 62 hardcodes the English string
Loading..., which bypassesnext-i18nextand won't be translated. The sibling branch at L49-56 already usesfoodProductViewTexts.loading— reuse it here. Additionally, rendering two near-identical loading UIs in sequence (once forisLoading, then again forisEditing === null) can cause a brief UI flicker; merging them would be cleaner.🌐 Proposed fix
- if (isLoading) { + if (isLoading || isEditing === null) { return ( <div className="flex items-center justify-center py-8"> <div className="size-8 animate-spin rounded-full border-b-2 border-blue-500"></div> <span className="ml-2">{foodProductViewTexts.loading}</span> </div> ); } - - if (isEditing === null) { - return ( - <div className="flex items-center justify-center py-8"> - <div className="size-8 animate-spin rounded-full border-b-2 border-blue-500"></div> - <span className="ml-2">Loading...</span> - </div> - ); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/components/Applications/FoodProduct/FoodProduct.tsx` around lines 49 - 65, The component renders two nearly identical loading UIs and hardcodes "Loading..." in the isEditing === null branch; update FoodProduct.tsx to consolidate the checks (use a single early return when isLoading || isEditing === null) and replace the hardcoded string with the i18n value foodProductViewTexts.loading so both cases show the same translated spinner/text; ensure you remove the duplicate branch and keep the existing spinner markup from the isLoading block.user/src/components/Applications/StageOptions/StageOptions.tsx (1)
38-44:⚠️ Potential issue | 🟡 MinorUse i18n for the "Loading..." label and consider merging with the
isLoadingbranch.Line 43 hardcodes
Loading...while the immediately preceding branch usesstageOptionTexts.general.loading. This breaks localization and produces a flicker when transitioning fromisLoading→isEditing === null.🌐 Proposed fix
- if (isLoading) { + if (isLoading || isEditing === null) { return <div>{stageOptionTexts.general.loading}</div>; } - - if (isEditing === null) { - return <div>Loading...</div>; - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/components/Applications/StageOptions/StageOptions.tsx` around lines 38 - 44, The component StageOptions currently returns a hardcoded "Loading..." when isEditing === null which breaks i18n and can flicker relative to the earlier isLoading branch; update the second branch to use the same localized string (stageOptionTexts.general.loading) or merge the conditions so both isLoading and isEditing === null render the same localized loading state; locate the conditional checks for isLoading and isEditing in the StageOptions component and replace the hardcoded text with stageOptionTexts.general.loading (or combine into a single if (isLoading || isEditing === null) return ...).
🧹 Nitpick comments (12)
user/src/components/Applications/Power/components/PowerFormView.tsx (1)
37-37: Tailwind クラスでの表示切替を推奨。
style={{ display: showForm ? 'block' : 'none' }}の代わりに、Tailwind の条件付きクラスを利用するとコーディングガイドラインに沿います。♻️ Proposed change
- <div style={{ display: showForm ? 'block' : 'none' }}> + <div className={showForm ? 'block' : 'hidden'}>As per coding guidelines: 「React コンポーネントのスタイルは Tailwind CSS を使用する」.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/components/Applications/Power/components/PowerFormView.tsx` at line 37, Replace the inline style display toggle in the PowerFormView component with Tailwind conditional classes: locate the div that currently uses style={{ display: showForm ? 'block' : 'none' }} and change it to use a className expression that applies Tailwind's "hidden" when showForm is false and "block" (or the appropriate layout class) when true, keeping any existing classes intact so styling follows the project's Tailwind-based convention.user/src/components/Applications/FireEquipment/components/FireEquipmentFormView.tsx (1)
60-60: Tailwind クラスでの表示切替を推奨。インラインの
style={{ display: ... }}ではなく Tailwind のhidden/blockを条件付きクラスで切り替える方がコーディングガイドラインに沿います。動作は等価です。♻️ Proposed change
- <div style={{ display: isRegister ? 'none' : 'block' }}> + <div className={isRegister ? 'hidden' : 'block'}> ... - <div style={{ display: isRegister ? 'block' : 'none' }}> + <div className={isRegister ? 'block' : 'hidden'}>As per coding guidelines: 「React コンポーネントのスタイルは Tailwind CSS を使用する」.
Also applies to: 71-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/components/Applications/FireEquipment/components/FireEquipmentFormView.tsx` at line 60, Replace the inline style display toggles in the FireEquipmentFormView component (the divs currently using style={{ display: isRegister ? 'none' : 'block' }} and the similar occurrence) with conditional Tailwind classes — e.g. compute className to use 'hidden' when isRegister is true and 'block' when false (use your project's classNames utility or a template literal) so the visibility follows the coding guideline of using Tailwind instead of inline style.user/src/components/Applications/Stage/StageForm/StageForm.tsx (1)
98-101: 既存のローディング文言を再利用してください。Line 100 は同じファイル内で既に使っている
stageFormTexts.loadingに寄せると、表示文言の二重管理を避けられます。♻️ Proposed refactor
{isFormMode === null ? ( <div className="w-[400px] py-4 text-center"> - <p>データを読み込み中です...</p> + <p>{stageFormTexts.loading}</p> </div>この UI 差分に対応する Storybook / before-after スクリーンショットの更新も確認してください。As per coding guidelines,
Update Storybook when making UI changes in frontend components. Based on learnings,UI 変更時は before/after のスクリーンショットを添付し、該当面のレビュアーをアサインする.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/components/Applications/Stage/StageForm/StageForm.tsx` around lines 98 - 101, The hardcoded loading message inside the isFormMode null branch should be replaced with the existing localized constant stageFormTexts.loading to avoid duplicate strings; update the JSX branch that renders the loading placeholder (the block using isFormMode === null) to render stageFormTexts.loading instead of the literal "データを読み込み中です...", keep the same container markup and CSS classes, and after changing the text update the Storybook stories and before/after screenshots for this StageForm variant and assign the relevant UI reviewer per guidelines.user/src/components/Applications/Group/GroupForm/hooks.ts (1)
19-21: Tighten the mutator prop types now that they're awaited.Lines 131, 142-144 now
awaitthese callbacks, but the parameter types (() => void) don't reflect that they return Promises. Callers presumably pass SWR'smutate(which returns a Promise) — update the types soawaitis meaningful and TS can catch non-async mistakes.♻️ Proposed change
- mutateGroups: () => void, - mutateCheckAllRegisteredGroups: () => void, - mutateGroupByUserId: () => void + mutateGroups: () => Promise<unknown>, + mutateCheckAllRegisteredGroups: () => Promise<unknown>, + mutateGroupByUserId: () => Promise<unknown>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/components/Applications/Group/GroupForm/hooks.ts` around lines 19 - 21, The mutator props mutateGroups, mutateCheckAllRegisteredGroups, and mutateGroupByUserId are declared as () => void but are awaited in callers (so they should return Promises); update their types to reflect async behavior (e.g., () => Promise<void> or () => Promise<unknown>) in the hook/props/type definition that declares these three symbols so TypeScript will require Promise-returning mutators (matching SWR's mutate).user/src/api/publicRelationsApi.ts (1)
48-52: Parameter type doesn't match the nullable-guard pattern used elsewhere.
groupIdis still typed asnumber, but the new guard treats0/undefined/NaNas "skip fetch". Sibling hooks were updated to widen the parameter type (number | nullinuseGetFoodProducts,number | undefinedinuseGetCookingProcessOrder). For consistency and to let TS callers actually pass the skip value, widen this one too.♻️ Proposed change
-export const usePublicRelationData = (groupId: number) => { +export const usePublicRelationData = (groupId: number | undefined) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/api/publicRelationsApi.ts` around lines 48 - 52, The parameter type for usePublicRelationData should be widened so callers can pass the "skip" sentinel used in the guard; change the signature of usePublicRelationData from (groupId: number) to (groupId: number | null) (or number | undefined if your codebase prefers undefined) and keep the existing endpoint null-check logic; update any direct callers if they currently pass null/undefined so TypeScript matches the intended nullable-guard behavior, referencing usePublicRelationData and the endpoint variable to locate the change.user/src/components/Applications/VenueMap/hooks.ts (1)
17-57:hasLoadedOncepermanently suppresses loading state after first completion.Once
hasLoadedOnceflips totrue,isLoadingreturned from this hook is alwaysfalse— even if the SWR key later changes (e.g., differentgroupId) and a fresh fetch begins. For this PR's tab-switch-preservation goal that is the intent, but if this hook is ever reused across group navigations, consumers will briefly render against the previousvenueMap. Consider resettinghasLoadedOncewhengroupIdchanges if that scenario matters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/components/Applications/VenueMap/hooks.ts` around lines 17 - 57, The current hasLoadedOnce flag is never reset so isLoading stays false after the first successful fetch; add a reset when the active group changes by introducing a new effect that calls setHasLoadedOnce(false) whenever groupId changes (e.g., useEffect(() => { setHasLoadedOnce(false); }, [groupId])); keep the existing effect that sets hasLoadedOnce true when isFetching becomes false so the combined behavior preserves the tab-switch intent but also re-enables loading for a new group.user/src/hooks/useApi.ts (1)
109-117: Global SWR defaults risk stale data across conditional endpoints — most callers only checkisLoading, notisValidating.Setting
keepPreviousData: trueglobally means callers with conditional endpoints (e.g.,groupId-dependent URLs) will momentarily render the previous key's cached data while fetching new data. Inspection of call sites shows this pattern is widespread:
useGetStageOrders,useGetVenueMap,useGetFoodProductsall use conditional endpoints based on parameter changes and only checkisLoading, notisValidating- Since SWR's
isLoadingreflects only the initial mount, these callers will display stale data (old group's data, etc.) until the new request completesThis is intentional for tab-switch stability but requires all callers to adopt defensive checks (e.g.,
isValidatingor request/response tracking) to avoid rendering wrong data during navigation. Either require this in documentation/patterns, or reconsider global defaults in favor of opt-in per hook.
dedupingInterval: 10000is also a sizeable window; ensure call sites don't assume automatic re-fetch within ~10s (explicitmutate()bypasses this and works as expected).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/hooks/useApi.ts` around lines 109 - 117, The global SWR defaults in useApi.ts are causing stale data for conditional endpoints; change the swrOptions (used in the return useSWR<T, ApiError>(key, authenticatedGetFetcher, swrOptions)) to remove or set keepPreviousData to false by default and reduce or remove the large dedupingInterval (10000) so callers relying only on isLoading won't render previous-key data; instead make keepPreviousData and dedupingInterval opt-in via the options spread (i.e., preserve ...options to allow callers to enable them explicitly) and document that callers needing previous-data behavior must pass those options into the useApi hook.user/src/components/Applications/Group/hooks.ts (1)
65-95: Consider extracting the sharedhasLoadedOnce+ one-timeisEditinginitialization into a reusable hook.This exact pattern (tri-state
isEditing,hasInitializedEditingref,hasLoadedOncegate, and functionaltoEdit) is now duplicated acrossGroup/hooks.ts,FoodProduct/hooks.ts,ViceRepresentative/hook.ts,StageOptions/hooks.ts, and siblings. Centralizing it would:
- Prevent subtle drift (e.g.,
Group/hooks.tsgates onisGroupResolvedbut the other hooks don't — easy to miss when evolving behavior).- Simplify future changes such as adding i18n loading labels or handling the "
isRegisterednever resolves" edge case in one place.♻️ Example extraction sketch
// user/src/hooks/useEditingMode.ts export const useEditingMode = ( isLoading: boolean, isRegistered?: boolean, isResolved = true ) => { const [isEditing, setIsEditing] = useState<boolean | null>(null); const [hasLoadedOnce, setHasLoadedOnce] = useState(false); const initialized = useRef(false); useEffect(() => { if (!isLoading) setHasLoadedOnce(true); }, [isLoading]); useEffect(() => { if (initialized.current || !isResolved || isRegistered === undefined) return; setIsEditing(!isRegistered); initialized.current = true; }, [isRegistered, isResolved]); const toEdit = () => setIsEditing((prev) => !prev); return { isEditing, toEdit, isLoading: isLoading && !hasLoadedOnce }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/components/Applications/Group/hooks.ts` around lines 65 - 95, Extract the duplicated editing/loading logic into a reusable hook (e.g., useEditingMode) and replace the inline state/refs in Group hooks: move isEditing state, hasInitializedEditing ref, hasLoadedOnce state, the two useEffects and toEdit into the new hook signature like useEditingMode(isLoading, isRegistered, isGroupResolved) returning { isEditing, toEdit, isLoading: isLoading && !hasLoadedOnce }; update the Group component to call useEditingMode and use its returned symbols instead of local isEditing, setIsEditing, hasInitializedEditing, hasLoadedOnce and the two effects so all files (Group/hooks.ts, FoodProduct/hooks.ts, ViceRepresentative/hook.ts, StageOptions/hooks.ts, etc.) can reuse the same logic and behavior.user/src/components/Applications/PublicRelations/hooks.ts (1)
71-88: Consider aligning init-guard with sibling hook for consistency.Nit:
user/src/components/Applications/CookingProcessOrder/hooks.tsgates the one-timeisEditinginitialization on!isDataLoadingas well asisRegistered !== undefined, while this hook only checksisRegistered. The current behavior is still correct (Content shows theisLoadingbranch before data arrives), but mirroring the same pattern across hooks reduces cognitive load and future drift.Also note
toEditrelying on(prev) => !prevyieldstruewhenprev === null; safe today because the edit button isn't rendered whileisEditing === null, but a more defensive form would besetIsEditing((prev) => prev === null ? prev : !prev).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/components/Applications/PublicRelations/hooks.ts` around lines 71 - 88, The one-time init guard in the useEffect that sets isEditing should mirror the sibling hook by also checking prIsLoading: inside the effect that references hasInitializedEditing.current and isRegistered, add a condition to return early when prIsLoading is true so initialization only runs after data finishes loading; also make the toEdit toggle defensive by changing its updater to avoid flipping when isEditing is null (use setIsEditing(prev => prev === null ? prev : !prev)). Ensure you update the references to hasInitializedEditing, isRegistered, prIsLoading, setIsEditing, and toEdit in this file.user/src/components/Applications/CookingProcessOrder/hooks.ts (1)
170-186: Initialization effect looks correct; one small robustness note.The one-time init pattern (
hasInitializedEditingref +!isDataLoadinggate) is sound. Minor:handleEditClickuses(prev) => !prev, which flipsnulltotrue. Today the edit button is not rendered whileisEditing === null, so this is unreachable — but making it explicit (prev === null ? prev : !prev) would harden it against future UI changes where the button could render during loading.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/components/Applications/CookingProcessOrder/hooks.ts` around lines 170 - 186, The init effect is fine but harden handleEditClick to avoid flipping a null isEditing: update the toggle handler (handleEditClick) to check for null and return prev as-is when prev === null (e.g. return prev === null ? prev : !prev) so clicking the edit button during loading/unknown state cannot flip null to true; reference hasInitializedEditing, isEditing, setIsEditing and handleEditClick when making this change.user/src/pages/home/index.tsx (2)
72-308: Heavy duplication across category branches — consider a config-driven renderer.All six
groupCategoryIdbranches repeat the same per-app components (VenueApplications,RentItems,Power,PublicRelations,VenueMap,FireEquipment, …) with identical prop wiring. Each addition/rename of an app-level prop must be repeated up to 6 times, which increases maintenance risk.Consider extracting small render helpers (e.g.
renderVenue(),renderRentItems(), …) or aRecord<groupCategoryId, AppKey[]>config that maps each category to the apps it should render. That would cut this block by hundreds of lines and eliminate the risk of missing one branch on future changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/pages/home/index.tsx` around lines 72 - 308, The JSX for different groupCategoryId branches duplicates the same app components (VenueApplications, RentItems, Power, PublicRelations, VenueMap, FireEquipment, FoodProduct, Stage, etc.) with identical prop wiring; replace the if/else branches with a config-driven renderer: create a mapping (e.g. Record<GROUP_CATEGORY, AppKey[]>) that lists which components to show per category and implement a small factory/renderer (e.g. renderVenue(), renderRentItems(), renderComponentByKey()) that returns each component with the shared props (groupId, groupCategoryId, isDeadline derived from userPageSettings, isRegistered from checkAllRegisteredGroups, mutateCheckAllRegisteredGroups when needed); then use that mapping keyed by groupCategoryId to produce the JSX, removing the repetitive conditional blocks and centralizing prop wiring for maintainability.
24-55: Import these types from the API modules instead of duplicating them.
UserPageSettingsis exported from@/api/userPageSettingAPI(line 7) but redefined locally. Similarly,CheckAllRegisteredGroupsmatches the exportedRegistrationStatusfrom@/api/checkAllRegisteredApi(line 4). Importing these types instead prevents silent drift when API shapes change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user/src/pages/home/index.tsx` around lines 24 - 55, The local type aliases UserPageSettings and CheckAllRegisteredGroups duplicate exported types from the API modules and risk drifting; remove the local definitions and import UserPageSettings from "@/api/userPageSettingAPI" and RegistrationStatus (the matching type) from "@/api/checkAllRegisteredApi" (or rename the imported RegistrationStatus to CheckAllRegisteredGroups where used) and update any references in this file (e.g., usages in components, props, or function signatures) to use the imported types instead of the redefined ones.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@user/src/api/foodProductApi.ts`:
- Around line 26-30: The current guard in useGetFoodProducts lets groupId === 0
produce a fetch to `${API_ENDPOINTS.FOOD_PRODUCTS_GROUP}/0`; update the guard so
falsy/invalid ids don't trigger a request — e.g., change the endpoint expression
in useGetFoodProducts to use a truthy check (groupId ?
`${API_ENDPOINTS.FOOD_PRODUCTS_GROUP}/${groupId}` : null) or only allow positive
ids (groupId > 0 ? ... : null); alternatively fix the caller
(useFoodProductHooks) to pass groupId ?? null instead of groupId || 0 so
useGetFoodProducts receives null for missing ids. Ensure you reference
useGetFoodProducts and API_ENDPOINTS.FOOD_PRODUCTS_GROUP when making the change.
In
`@user/src/components/Applications/CookingProcessOrder/CookingProcessOrder.tsx`:
- Around line 42-43: Replace the hardcoded "Loading..." placeholder with the
i18n label already provided by the hook: use
cookingProcessOrderTexts.general.loading (or t('general.loading')) where the
conditional renders when isLoading || isEditing === null; also apply the same
Tailwind loading placeholder classes used elsewhere in this component (match the
surrounding skeleton/placeholder styles) so the loading UI is consistent with
other components.
In `@user/src/components/Applications/Group/Group.tsx`:
- Around line 55-57: The null check branch currently returns a hardcoded
"Loading..." string; change it to reuse the existing i18n text by returning the
same localized label used elsewhere (groupTexts.loading) so both branches show
consistent localized text; locate the null branch in the Group component (the
isEditing === null check) and replace the raw string with groupTexts.loading.
In `@user/src/components/Applications/PublicRelations/PublicRelations.tsx`:
- Around line 44-46: The branch that returns a hardcoded <div>Loading...</div>
when isEditing === null should use the existing i18n string instead; replace
that hardcoded text with the localized string used elsewhere
(publicRelationsTexts.loading or t('general.loading')) inside the
PublicRelations component so the loading label is consistent with the rest of
the UI (locate the isEditing === null check in PublicRelations.tsx and return
the same localized value rather than the hardcoded English string).
In `@user/src/components/Applications/ViceRepresentative/ViceRepresentative.tsx`:
- Around line 49-51: The null-loading branch in the ViceRepresentative component
returns a hardcoded "Loading..." string; replace that with the i18n value used
elsewhere by returning viceRepresentativeTexts.general.loading instead. Locate
the isEditing === null check in ViceRepresentative.tsx and change the JSX
returned there to use the existing viceRepresentativeTexts.general.loading
identifier so the component consistently reuses the same localized loading text.
---
Outside diff comments:
In `@user/src/components/Applications/FoodProduct/FoodProduct.tsx`:
- Around line 49-65: The component renders two nearly identical loading UIs and
hardcodes "Loading..." in the isEditing === null branch; update FoodProduct.tsx
to consolidate the checks (use a single early return when isLoading || isEditing
=== null) and replace the hardcoded string with the i18n value
foodProductViewTexts.loading so both cases show the same translated
spinner/text; ensure you remove the duplicate branch and keep the existing
spinner markup from the isLoading block.
In `@user/src/components/Applications/Stage/hooks/useStageForm.ts`:
- Around line 80-103: The reset effect in useStageForm can overwrite user edits
because updateField calls setValue without updating formState.isDirty/isTouched;
modify the updateField function so its setValue call includes shouldDirty: true
and shouldTouch: true (in addition to shouldValidate: true) to ensure user edits
flip isDirty/isTouched and prevent the useEffect reset from running over
in-progress edits (look for updateField, setValue, and the useEffect that calls
reset).
In `@user/src/components/Applications/StageOptions/StageOptions.tsx`:
- Around line 38-44: The component StageOptions currently returns a hardcoded
"Loading..." when isEditing === null which breaks i18n and can flicker relative
to the earlier isLoading branch; update the second branch to use the same
localized string (stageOptionTexts.general.loading) or merge the conditions so
both isLoading and isEditing === null render the same localized loading state;
locate the conditional checks for isLoading and isEditing in the StageOptions
component and replace the hardcoded text with stageOptionTexts.general.loading
(or combine into a single if (isLoading || isEditing === null) return ...).
---
Nitpick comments:
In `@user/src/api/publicRelationsApi.ts`:
- Around line 48-52: The parameter type for usePublicRelationData should be
widened so callers can pass the "skip" sentinel used in the guard; change the
signature of usePublicRelationData from (groupId: number) to (groupId: number |
null) (or number | undefined if your codebase prefers undefined) and keep the
existing endpoint null-check logic; update any direct callers if they currently
pass null/undefined so TypeScript matches the intended nullable-guard behavior,
referencing usePublicRelationData and the endpoint variable to locate the
change.
In `@user/src/components/Applications/CookingProcessOrder/hooks.ts`:
- Around line 170-186: The init effect is fine but harden handleEditClick to
avoid flipping a null isEditing: update the toggle handler (handleEditClick) to
check for null and return prev as-is when prev === null (e.g. return prev ===
null ? prev : !prev) so clicking the edit button during loading/unknown state
cannot flip null to true; reference hasInitializedEditing, isEditing,
setIsEditing and handleEditClick when making this change.
In
`@user/src/components/Applications/FireEquipment/components/FireEquipmentFormView.tsx`:
- Line 60: Replace the inline style display toggles in the FireEquipmentFormView
component (the divs currently using style={{ display: isRegister ? 'none' :
'block' }} and the similar occurrence) with conditional Tailwind classes — e.g.
compute className to use 'hidden' when isRegister is true and 'block' when false
(use your project's classNames utility or a template literal) so the visibility
follows the coding guideline of using Tailwind instead of inline style.
In `@user/src/components/Applications/Group/GroupForm/hooks.ts`:
- Around line 19-21: The mutator props mutateGroups,
mutateCheckAllRegisteredGroups, and mutateGroupByUserId are declared as () =>
void but are awaited in callers (so they should return Promises); update their
types to reflect async behavior (e.g., () => Promise<void> or () =>
Promise<unknown>) in the hook/props/type definition that declares these three
symbols so TypeScript will require Promise-returning mutators (matching SWR's
mutate).
In `@user/src/components/Applications/Group/hooks.ts`:
- Around line 65-95: Extract the duplicated editing/loading logic into a
reusable hook (e.g., useEditingMode) and replace the inline state/refs in Group
hooks: move isEditing state, hasInitializedEditing ref, hasLoadedOnce state, the
two useEffects and toEdit into the new hook signature like
useEditingMode(isLoading, isRegistered, isGroupResolved) returning { isEditing,
toEdit, isLoading: isLoading && !hasLoadedOnce }; update the Group component to
call useEditingMode and use its returned symbols instead of local isEditing,
setIsEditing, hasInitializedEditing, hasLoadedOnce and the two effects so all
files (Group/hooks.ts, FoodProduct/hooks.ts, ViceRepresentative/hook.ts,
StageOptions/hooks.ts, etc.) can reuse the same logic and behavior.
In `@user/src/components/Applications/Power/components/PowerFormView.tsx`:
- Line 37: Replace the inline style display toggle in the PowerFormView
component with Tailwind conditional classes: locate the div that currently uses
style={{ display: showForm ? 'block' : 'none' }} and change it to use a
className expression that applies Tailwind's "hidden" when showForm is false and
"block" (or the appropriate layout class) when true, keeping any existing
classes intact so styling follows the project's Tailwind-based convention.
In `@user/src/components/Applications/PublicRelations/hooks.ts`:
- Around line 71-88: The one-time init guard in the useEffect that sets
isEditing should mirror the sibling hook by also checking prIsLoading: inside
the effect that references hasInitializedEditing.current and isRegistered, add a
condition to return early when prIsLoading is true so initialization only runs
after data finishes loading; also make the toEdit toggle defensive by changing
its updater to avoid flipping when isEditing is null (use setIsEditing(prev =>
prev === null ? prev : !prev)). Ensure you update the references to
hasInitializedEditing, isRegistered, prIsLoading, setIsEditing, and toEdit in
this file.
In `@user/src/components/Applications/Stage/StageForm/StageForm.tsx`:
- Around line 98-101: The hardcoded loading message inside the isFormMode null
branch should be replaced with the existing localized constant
stageFormTexts.loading to avoid duplicate strings; update the JSX branch that
renders the loading placeholder (the block using isFormMode === null) to render
stageFormTexts.loading instead of the literal "データを読み込み中です...", keep the same
container markup and CSS classes, and after changing the text update the
Storybook stories and before/after screenshots for this StageForm variant and
assign the relevant UI reviewer per guidelines.
In `@user/src/components/Applications/VenueMap/hooks.ts`:
- Around line 17-57: The current hasLoadedOnce flag is never reset so isLoading
stays false after the first successful fetch; add a reset when the active group
changes by introducing a new effect that calls setHasLoadedOnce(false) whenever
groupId changes (e.g., useEffect(() => { setHasLoadedOnce(false); },
[groupId])); keep the existing effect that sets hasLoadedOnce true when
isFetching becomes false so the combined behavior preserves the tab-switch
intent but also re-enables loading for a new group.
In `@user/src/hooks/useApi.ts`:
- Around line 109-117: The global SWR defaults in useApi.ts are causing stale
data for conditional endpoints; change the swrOptions (used in the return
useSWR<T, ApiError>(key, authenticatedGetFetcher, swrOptions)) to remove or set
keepPreviousData to false by default and reduce or remove the large
dedupingInterval (10000) so callers relying only on isLoading won't render
previous-key data; instead make keepPreviousData and dedupingInterval opt-in via
the options spread (i.e., preserve ...options to allow callers to enable them
explicitly) and document that callers needing previous-data behavior must pass
those options into the useApi hook.
In `@user/src/pages/home/index.tsx`:
- Around line 72-308: The JSX for different groupCategoryId branches duplicates
the same app components (VenueApplications, RentItems, Power, PublicRelations,
VenueMap, FireEquipment, FoodProduct, Stage, etc.) with identical prop wiring;
replace the if/else branches with a config-driven renderer: create a mapping
(e.g. Record<GROUP_CATEGORY, AppKey[]>) that lists which components to show per
category and implement a small factory/renderer (e.g. renderVenue(),
renderRentItems(), renderComponentByKey()) that returns each component with the
shared props (groupId, groupCategoryId, isDeadline derived from
userPageSettings, isRegistered from checkAllRegisteredGroups,
mutateCheckAllRegisteredGroups when needed); then use that mapping keyed by
groupCategoryId to produce the JSX, removing the repetitive conditional blocks
and centralizing prop wiring for maintainability.
- Around line 24-55: The local type aliases UserPageSettings and
CheckAllRegisteredGroups duplicate exported types from the API modules and risk
drifting; remove the local definitions and import UserPageSettings from
"@/api/userPageSettingAPI" and RegistrationStatus (the matching type) from
"@/api/checkAllRegisteredApi" (or rename the imported RegistrationStatus to
CheckAllRegisteredGroups where used) and update any references in this file
(e.g., usages in components, props, or function signatures) to use the imported
types instead of the redefined ones.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 90c27d79-ec2c-4198-bade-9bf60234530f
📒 Files selected for processing (30)
user/src/api/cookingProcessOrderApi.tsuser/src/api/foodProductApi.tsuser/src/api/publicRelationsApi.tsuser/src/components/Applications/CookingProcessOrder/CookingProcessOrder.tsxuser/src/components/Applications/CookingProcessOrder/hooks.tsuser/src/components/Applications/FireEquipment/components/FireEquipmentFormView.tsxuser/src/components/Applications/FoodProduct/FoodProduct.tsxuser/src/components/Applications/FoodProduct/FoodProductForm/hooks.tsuser/src/components/Applications/FoodProduct/hooks.tsuser/src/components/Applications/Group/Group.tsxuser/src/components/Applications/Group/GroupForm/GroupForm.tsxuser/src/components/Applications/Group/GroupForm/hooks.tsuser/src/components/Applications/Group/hooks.tsuser/src/components/Applications/Power/components/PowerFormView.tsxuser/src/components/Applications/Power/hooks/usePowerApplication.tsuser/src/components/Applications/PublicRelations/PublicRelations.tsxuser/src/components/Applications/PublicRelations/hooks.tsuser/src/components/Applications/Stage/StageForm/StageForm.tsxuser/src/components/Applications/Stage/hooks/useStageForm.tsuser/src/components/Applications/Stage/hooks/useStageFormViewHooks.tsuser/src/components/Applications/StageOptions/StageOptionForm/StageOptionForm.tsxuser/src/components/Applications/StageOptions/StageOptionForm/hooks.tsuser/src/components/Applications/StageOptions/StageOptions.tsxuser/src/components/Applications/StageOptions/hooks.tsuser/src/components/Applications/VenueApplication/hooks.tsuser/src/components/Applications/VenueMap/hooks.tsuser/src/components/Applications/ViceRepresentative/ViceRepresentative.tsxuser/src/components/Applications/ViceRepresentative/hook.tsuser/src/hooks/useApi.tsuser/src/pages/home/index.tsx
💤 Files with no reviewable changes (1)
- user/src/components/Applications/FoodProduct/FoodProductForm/hooks.ts
KokiWakatsuki
approved these changes
Apr 24, 2026
This file contains hidden or 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
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.
対応 Issue
resolve #1973
前提
この PR は、ユーザー画面の各種申請フォームで「画面遷移・タブ移動・アコーディオンの開閉・再取得」のタイミングで入力内容が消える問題を直すための修正です。
対象は
user/src/pages/home/index.tsxに表示される申請群です。特定のフォームだけではなく、複数の申請で同じような実装パターンがあり、同じ種類の入力リセットが起きる可能性がありました。今回
gm3/developを取り込んだことで、火器使用申請のユーザー画面もこの PR の対象に含めています。何が起きていたか
主な原因は、フォームの表示状態やデータ再取得により React コンポーネントが再生成・再初期化されることでした。
対応方針
「フォームを必要以上に壊さない」ことを優先して修正しています。
isEditingをboolean | nullとして扱うisRegisteredを受け取り、登録済みかどうかに応じた edit/view の初期化を一度だけ行うuseMemoやコピーを使い、参照や再生成によるリセットを避けるgroupIdが未確定または0のときに不要な API fetch をしないようにする主な変更点
toEditで表示モードへ戻るようにしました。home/index.tsxではカテゴリ別の申請表示を整理し、火器使用申請も該当カテゴリの流れに含めました。Loading...の直書きは、既存の i18n 文言に寄せました。火器使用申請について
火器使用申請は、
gm3/develop側で追加済みの機能をこの PR に取り込んだうえで確認しました。確認した問題:
対応:
FireEquipmentFormViewの使用/不使用切り替えを CSS 表示切り替えに変更レビュー観点
1. まず表示順と対象申請を確認
2. 未登録フォームの初期表示を確認
groupId未確定時に/group/0のような不要 API fetch が発生しない3. 入力保持を確認
各フォームで、入力途中に別申請を開く・閉じる・戻る操作をしてください。
4. コピペ操作を確認
今回の Issue で特に見たい観点です。
5. submit 成功/失敗時の挙動を確認
6. 火器使用申請を重点確認
ローカル確認結果
docker compose run --rm user pnpm run type-checkdocker compose run --rm user pnpm run lintgm3/developを取り込み済み補足
docker compose run --rm api rails testは実行しましたが、api/test/fixtures/assign_rental_items.ymlが現在の schema に存在しない旧カラムrental_order_id/rentable_item_idを使っているため、fixture 読み込み時点で失敗します。この fixture 不整合は今回のフォーム状態管理の変更とは別件で、
gm3/develop側にもある既存のテスト環境問題です。