fix: restore location when navigates from a nested route to a new tab

[tomrndom]

ref: https://app.clickup.com/t/86b8d9fdp

Signed-off-by: Tomás Castillo [email protected]

Summary by CodeRabbit

• Refactor
  • Tab navigation now syncs with URL fragments, initializes a default fragment when missing, and better handles nested form routes (push vs replace behavior).
• UX
  • Nested form routes display a breadcrumb and preserve intended tab focus; "Manage Items" navigation includes a fragment to target the correct tab.
• Tests
  • Expanded coverage for fragment-based tab behavior, nested-route scenarios, and updated test scaffolding.
• Chores
  • Made form-list default state publicly accessible for reuse.

[coderabbitai]

📝 Walkthrough

Walkthrough

Reworks tab ↔ URL-fragment handling on the sponsor edit page: introduces getTabFromFragment(location), derives nested-route awareness from match.params.form_id, synchronizes selected tab with location.hash via effects, initializes a default hash when empty, and routes-aware tab changes using history.push for nested routes or history.replace for same-path navigation.

Changes

Cohort / File(s)	Summary
Sponsor Edit Navigation src/pages/sponsors/edit-sponsor-page.js	Replaced getTabFromUrlFragment with getTabFromFragment(location); moved hash sync into React effects; detect nested form via match.params.form_id; initialize default hash when missing; change tab navigation to history.push for nested routes or history.replace for same-path hash updates; removed window.hashchange listener and per-tab onClick handlers.
Tests — EditSponsorPage src/pages/sponsors/__tests__/edit-sponsor-page.test.js	Renamed imports to getTabFromFragment; added mocks for sponsor action creators; included currentSponsorFormsDefaultState; updated tests to assert history.replace for same-path tab changes and history.push for nested-route tab navigation; adjusted fragment-change and state initialization tests.
Reducer export src/reducers/sponsors/sponsor-page-forms-list-reducer.js	Made DEFAULT_STATE a named export (export const DEFAULT_STATE) so tests/consumers can import the default forms-list state.
Routing / Layout src/layouts/sponsor-id-layout.js	Changed sponsor-forms/:form_id/items route to use the render prop and wrap EditSponsorPage with a Breadcrumb for the nested form-items route.
Forms Tab navigation target src/pages/sponsors/sponsor-forms-tab/index.js	Updated Manage Items link to include #forms fragment in the URL when navigating to item-items view.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Tabs as Tabs(UI)
    participant Edit as EditSponsorPage
    participant Router as History/Router
    participant Location as BrowserLocation

    User->>Tabs: select tab (e.g., "forms")
    Tabs->>Edit: onChange(fragment)
    alt nested form route active (match.params.form_id)
        Edit->>Router: history.push("/app/summits/.../sponsors/:id/sponsor-forms/:form_id/items#forms")
        Router->>Location: update URL (push)
    else same-path
        Edit->>Router: history.replace(currentPath + "#forms")
        Router->>Location: update hash (replace)
    end
    Location->>Edit: location.hash updated
    Edit->>Tabs: useEffect sync selectedTab from location.hash

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• 86b82fa34 - feat: add url fragment linking to tabs on edit sponsor page #751 — Related refactor of fragment-based tab linking and hash handling in EditSponsorPage.
• feat: sponsor purchases tab #757 — Touches EditSponsorPage tab panel composition (adds SponsorPurchasesTab); may interact with tab/fragment mapping.
• feat: sponsor cart view #748 — Modifies EditSponsorPage tab handling and tab-to-fragment mappings.

Suggested reviewers

• smarcet

Poem

🐰 I hopped through hashes and tabs with delight,
If nested I push, if simple I replace in one flight.
Breadcrumbs show forms tucked in a lane,
Tabs and fragments now dance in refrain —
A rabbit applauds this tidy new sight!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly relates to the main change: refactoring tab navigation to restore location state when navigating from nested routes, which is the core objective of this pull request.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/sponsor-nested-route-fix

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[smarcet]

@tomrndom

summit-admin/src/pages/sponsors/edit-sponsor-page.js

Line 185 in a8672e8

const sponsorFormItemRoute =

This string-matching against location.pathname is fragile and bypasses React `Router entirely. The app already has two separate entries in sponsor-id-layout.js:144-150 that render
EditSponsorPage:

<Switch>
  <Route exact path={match.url} component={EditSponsorPage}/>
  <Route
    path={`${match.url}/sponsor-forms/:form_id/items`}
    component={EditSponsorPage}
  />
</Switch>

The nested route already captures :form_id as a route param. The proper fix is to use match.params:

const sponsorFormItemRoute = !!match.params?.form_id;

This eliminates the URL sniffing entirely and leverages React Router as intended.

[smarcet]

just use a variable
const isNestedFormItemRoute = !!match.params?.form_id;
u could use it to at https://github.com/fntechgit/summit-admin/pull/781/changes#r2775041275
and then this method could be simple

const handleTabChange = (event, newValue) => {
setSelectedTab(newValue);
const fragment = getFragmentFromValue(newValue);
if (match.params?.form_id) {
history.push(
/app/summits/${currentSummit.id}/sponsors/${entity.id}#${fragment}
);
} else {
history.replace({ ...location, hash: #${fragment} });
}
}

[smarcet]

@tomrndom here

summit-admin/src/pages/sponsors/edit-sponsor-page.js

Line 208 in a8672e8

onClick={() => handleTabChange(null, t.value)}

 <Tabs onChange={handleTabChange}>

already fires handleTabChange when a child is clicked. The additional onClick on each causes handleTabChange to fire twice per click, which now
means a double history.push on nested routes.

[smarcet]

react router already provide these props

 const {                                                                                                                                                                                           
      entity,                                     
      member,                                                                                                                                                                                       
      history,                                                                                                                                                                                      
      location,  // ← React Router provides this                                                                                                                                                    
      match,     // ← and this           
      ...
  } = props;

use location react router instead of windows.location
so here

summit-admin/src/pages/sponsors/edit-sponsor-page.js

Line 62 in a8672e8

export const getTabFromUrlFragment = () => {

refactor it
to

  export const getTabFromFragment = (location) => {                  
    const currentHash = (location.hash || "").replace("#", "");                                                                                                                                     
    const result = tabsToFragmentMap.indexOf(currentHash);         
    if (result > -1) return result;                                                                                                                                                                 
    return 0;                                                                                                                                                                                       
  };   

and here

summit-admin/src/pages/sponsors/edit-sponsor-page.js

Line 137 in a8672e8

useEffect(() => {

lets refactor to

 useEffect(() => {             
     setSelectedTab(getTabFromUrlFragment(location));                                                                                                                                              
   }, [location.hash]);                 
  
  useEffect(() => {
     if (!location.hash) {
       history.replace({ ...location, hash: `#${getFragmentFromValue(0)}` });
     }
  }, []);

This way:

• First useEffect — syncs tab state whenever the hash changes (React Router handles all navigation events)
• Second useEffect — on mount only, sets the default #general hash in the URL if none exists, without calling handleTabChange or relying on stale closures

[smarcet]

@tomrndom please review comments

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/pages/sponsors/__tests__/edit-sponsor-page.test.js`:
- Around line 83-100: The first test uses a nested shape for
currentSummitSponsorshipListState (an object with a sponsorships property
containing the list and pagination) but the reducer expects a flat shape; update
the test's state for currentSummitSponsorshipListState to the flat structure
(sponsorships, currentPage, lastPage, perPage, order, orderDir,
totalSponsorships) to match the reducer and the second test; ensure
currentSponsorState and sponsorPageFormsListState remain unchanged.

🧹 Nitpick comments (2)

src/pages/sponsors/edit-sponsor-page.js (1)

193-193: Remove duplicate variable.

sponsorFormItemRoute is identical to isNestedFormItemRoute defined at line 123. Use the existing variable instead.

♻️ Suggested fix

-  const sponsorFormItemRoute = !!match.params?.form_id;
-
   return (
     <Box>
       ...
         <CustomTabPanel value={selectedTab} index={4}>
-          {sponsorFormItemRoute ? (
+          {isNestedFormItemRoute ? (
             <SponsorFormsManageItems match={match} />

src/pages/sponsors/__tests__/edit-sponsor-page.test.js (1)

105-105: Misleading variable name.

The variable is named usersTabReference but it selects "edit_sponsor.tab.forms". Rename to formsTabReference for clarity.

♻️ Suggested fix

-      const usersTabReference = screen.getByText("edit_sponsor.tab.forms");
+      const formsTabReference = screen.getByText("edit_sponsor.tab.forms");

       await act(async () => {
-        await userEvent.click(usersTabReference);
+        await userEvent.click(formsTabReference);
       });