Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tomrndom wants to merge 5 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b04f52e
fix: restore location when the user navigates from a nested route to …
tomrndom Feb 5, 2026
a8672e8
fix: adjust tests and add new case for nested routes
tomrndom Feb 5, 2026
178d335
fix: address comments, adjust navigation using history and react router
tomrndom Feb 6, 2026
6e61e0a
fix: update tests with history and react-router
tomrndom Feb 6, 2026
82aa709
fix: adjust reducer structure on test case
tomrndom Feb 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/layouts/sponsor-id-layout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,17 @@ class SponsorIdLayout extends React.Component {
<Route exact path={match.url} component={EditSponsorPage} />
<Route
path={`${match.url}/sponsor-forms/:form_id/items`}
component={EditSponsorPage}
render={(props) => (
<div>
<Breadcrumb
data={{
title: "Forms",
pathname: `${match.url}`
}}
/>
<EditSponsorPage {...props} />
</div>
)}
/>
</Switch>
<Route component={NoMatchPage} />
Expand Down
164 changes: 92 additions & 72 deletions src/pages/sponsors/__tests__/edit-sponsor-page.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,32 @@ import userEvent from "@testing-library/user-event";
import { act, screen } from "@testing-library/react";
import EditSponsorPage, {
getFragmentFromValue,
getTabFromUrlFragment
getTabFromFragment
} from "../edit-sponsor-page";
import { renderWithRedux } from "../../../utils/test-utils";
import { DEFAULT_STATE as currentSponsorDefaultState } from "../../../reducers/sponsors/sponsor-reducer";
import { DEFAULT_STATE as currentSponsorFormsDefaultState } from "../../../reducers/sponsors/sponsor-page-forms-list-reducer";
import {
DEFAULT_ENTITY as defaultSummitEntity,
DEFAULT_STATE as currentSummitDefaultState
} from "../../../reducers/summits/current-summit-reducer";

global.window = { location: { pathname: "/sponsor-forms/items" } };
jest.mock(
"../sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.js"
);
jest.mock("../sponsor-users-list-per-sponsor/index.js");

jest.mock("../../../actions/sponsor-actions", () => ({
...jest.requireActual("../../../actions/sponsor-actions"),
getSponsorAdvertisements: jest.fn(() => ({ type: "MOCK_ACTION" })),
getSponsorMaterials: jest.fn(() => ({ type: "MOCK_ACTION" })),
getSponsorSocialNetworks: jest.fn(() => ({ type: "MOCK_ACTION" })),
getSponsorLeadReportSettingsMeta: jest.fn(() => ({ type: "MOCK_ACTION" })),
getSponsorTiers: jest.fn(() => ({ type: "MOCK_ACTION" })),
getExtraQuestionMeta: jest.fn(() => ({ type: "MOCK_ACTION" })),
resetSponsorForm: jest.fn(() => ({ type: "MOCK_ACTION" }))
}));

describe("EditSponsorPage", () => {
describe("getFragmentFromValue", () => {
it("returns correct values", () => {
Expand All @@ -35,61 +46,34 @@ describe("EditSponsorPage", () => {
});
});

describe("getTabFromUrlFragment", () => {
describe("getTabFromFragment", () => {
it("returns correct values for defined fragments", () => {
const newUrl1 = "#general";
window.location.hash = newUrl1;

const result1 = getTabFromUrlFragment();
expect(result1).toBe(0);

const newUrl2 = "#pages";
window.location.hash = newUrl2;

const result2 = getTabFromUrlFragment();
expect(result2).toBe(2);

const newUrl3 = "#media_uploads";
window.location.hash = newUrl3;

const result3 = getTabFromUrlFragment();
expect(result3).toBe(3);

const newUrl4 = "#badge_scans";
window.location.hash = newUrl4;

const result4 = getTabFromUrlFragment();
expect(result4).toBe(7);
expect(getTabFromFragment({ hash: "#general" })).toBe(0);
expect(getTabFromFragment({ hash: "#users" })).toBe(1);
expect(getTabFromFragment({ hash: "#pages" })).toBe(2);
expect(getTabFromFragment({ hash: "#media_uploads" })).toBe(3);
expect(getTabFromFragment({ hash: "#forms" })).toBe(4);
expect(getTabFromFragment({ hash: "#badge_scans" })).toBe(7);
});
});

describe("Component", () => {
const originalWindowLocation = window.location;
it("should change the url fragment on tab click", async () => {
delete window.location;

Object.defineProperty(window, "location", {
configurable: true,
writable: true,
value: {
...originalWindowLocation,
hash: "#general"
}
});
it("should change the url fragment on tab click (same path)", async () => {
const mockHistory = { push: jest.fn(), replace: jest.fn() };

renderWithRedux(
<EditSponsorPage
history={{}}
history={mockHistory}
location={{
pathname: "/sponsor-forms/items"
pathname: "/app/summits/12/sponsors/123"
}}
match={{}}
/>,
{
initialState: {
currentSummitState: {
currentSummit: defaultSummitEntity,
...currentSummitDefaultState
...currentSummitDefaultState,
currentSummit: { ...defaultSummitEntity, id: 12 }
},
loggedUserState: {
member: {
Expand All @@ -106,8 +90,11 @@ describe("EditSponsorPage", () => {
totalSponsorships: 0
},
currentSponsorState: {
sponsorships: [],
...currentSponsorDefaultState
...currentSponsorDefaultState,
entity: { ...currentSponsorDefaultState.entity, id: 123 }
},
sponsorPageFormsListState: {
...currentSponsorFormsDefaultState
}
}
}
Expand All @@ -119,26 +106,69 @@ describe("EditSponsorPage", () => {
await userEvent.click(usersTabReference);
});

expect(window.location.hash).toBe("forms");
expect(mockHistory.replace).toHaveBeenCalledWith(
expect.objectContaining({
hash: "#forms"
})
);
expect(mockHistory.push).not.toHaveBeenCalled();
});

it("should change the tab rendered on fragment change", async () => {
delete window.location;
it("should call history.push on tab click when on nested route", async () => {
const mockHistory = { push: jest.fn(), replace: jest.fn() };

Object.defineProperty(window, "location", {
configurable: true,
writable: true,
value: {
...originalWindowLocation,
hash: "#general"
renderWithRedux(
<EditSponsorPage
history={mockHistory}
location={{
pathname: "/app/summits/12/sponsors/44/sponsor-forms/15/items"
}}
match={{ params: { form_id: 15 } }}
/>,
{
initialState: {
currentSummitState: {
...currentSummitDefaultState,
currentSummit: { ...defaultSummitEntity, id: 12 }
},
loggedUserState: {
member: { groups: {} }
},
currentSummitSponsorshipListState: {
sponsorships: [],
currentPage: 1,
lastPage: 1,
perPage: 100,
order: "order",
orderDir: 1,
totalSponsorships: 0
},
currentSponsorState: {
...currentSponsorDefaultState,
entity: { ...currentSponsorDefaultState.entity, id: 44 }
}
}
}
);

const usersTab = screen.getByText("edit_sponsor.tab.users");

await act(async () => {
await userEvent.click(usersTab);
});

renderWithRedux(
expect(mockHistory.push).toHaveBeenCalledWith(
"/app/summits/12/sponsors/44#users"
);
});

it("should change the tab rendered on fragment change", () => {
const { rerender } = renderWithRedux(
<EditSponsorPage
history={{}}
location={{
pathname: "/sponsor-forms/items"
pathname: "/sponsor-forms/items",
hash: "#general"
}}
match={{}}
/>,
Expand Down Expand Up @@ -173,26 +203,16 @@ describe("EditSponsorPage", () => {
const generalTabPanel = screen.getByTestId("simple-tabpanel-0");
expect(generalTabPanel).toBeDefined();

delete window.location;

Object.defineProperty(window, "location", {
configurable: true,
writable: true,
value: {
...originalWindowLocation,
hash: "#users"
}
});
rerender(
<EditSponsorPage
history={{ replace: jest.fn() }}
location={{ pathname: "/x", hash: "#users" }}
match={{}}
/>
);

const usersTabPanel = screen.getByTestId("simple-tabpanel-1");
expect(usersTabPanel).toBeDefined();
});

afterEach(() => {
Object.defineProperty(window, "location", {
configurable: true,
value: originalWindowLocation
});
});
});
});
40 changes: 27 additions & 13 deletions src/pages/sponsors/edit-sponsor-page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ export const tabsToFragmentMap = [

export const getFragmentFromValue = (index) => tabsToFragmentMap[index];

export const getTabFromUrlFragment = () => {
const currentHash = window.location.hash.replace("#", "");
export const getTabFromFragment = (location) => {
const currentHash = (location.hash || "").replace("#", "");
const result = tabsToFragmentMap.indexOf(currentHash);
if (result > -1) return result;
return 0;
Expand Down Expand Up @@ -118,19 +118,36 @@ const EditSponsorPage = (props) => {
getExtraQuestionMeta
} = props;

const [selectedTab, setSelectedTab] = useState(getTabFromUrlFragment());
const [selectedTab, setSelectedTab] = useState(getTabFromFragment(location));

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

const handleTabChange = (event, newValue) => {
Copy link

@smarcet smarcet Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

setSelectedTab(newValue);
window.location.hash = getFragmentFromValue(newValue);
const fragment = getFragmentFromValue(newValue);
if (isNestedFormItemRoute) {
history.push(
`/app/summits/${currentSummit.id}/sponsors/${entity.id}#${fragment}`
);
} else {
history.replace({ ...location, hash: `#${fragment}` });
}
};

useEffect(() => {
const onHashChange = () => setSelectedTab(getTabFromUrlFragment());
window.addEventListener("hashchange", onHashChange);
// default call
if (!window.location.hash) handleTabChange(null, getTabFromUrlFragment());
return () => window.removeEventListener("hashchange", onHashChange);
setSelectedTab(getTabFromFragment(location));
}, [location.hash]);

useEffect(() => {
if (!location.hash) {
const defaultTab = isNestedFormItemRoute
? SPONSOR_TABS.FORMS
: SPONSOR_TABS.GENERAL;
history.replace({
...location,
hash: `#${getFragmentFromValue(defaultTab)}`
});
}
}, []);

useEffect(() => {
Expand Down Expand Up @@ -173,9 +190,7 @@ const EditSponsorPage = (props) => {
}
];

const sponsorFormItemRoute =
location.pathname.includes("/sponsor-forms/") &&
location.pathname.includes("/items");
const sponsorFormItemRoute = !!match.params?.form_id;

return (
<Box>
Expand All @@ -196,7 +211,6 @@ const EditSponsorPage = (props) => {
key={t.value}
label={t.label}
value={t.value}
onClick={() => handleTabChange(null, t.value)}
sx={{
fontSize: "1.4rem",
lineHeight: "1.8rem",
Expand Down
2 changes: 1 addition & 1 deletion src/pages/sponsors/sponsor-forms-tab/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ const SponsorFormsTab = ({

const handleManageItems = (item) => {
history.push(
`/app/summits/${summitId}/sponsors/${sponsor.id}/sponsor-forms/${item.id}/items`
`/app/summits/${summitId}/sponsors/${sponsor.id}/sponsor-forms/${item.id}/items#forms`
);
};

Expand Down
2 changes: 1 addition & 1 deletion src/reducers/sponsors/sponsor-page-forms-list-reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
} from "../../actions/sponsor-forms-actions";
import { SET_CURRENT_SUMMIT } from "../../actions/summit-actions";

const DEFAULT_STATE = {
export const DEFAULT_STATE = {
managedForms: {
forms: [],
order: "name",
Expand Down