Skip to content

Commit

Permalink
fix navigation links
Browse files Browse the repository at this point in the history
  • Loading branch information
macfarlandian committed Jan 7, 2021
1 parent 6a575ed commit 0bd1282
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 25 deletions.
2 changes: 1 addition & 1 deletion spotlight-client/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const App: React.FC = () => {
<PageTenant path="/" />
<PassThroughPage path={`/${NarrativesSlug}`}>
<PageNarrativeList path="/" />
<PageNarrative path="/:narrativeTypeId" />
<PageNarrative path="/:narrativeTypeId/*sectionNumber" />
</PassThroughPage>
<PageNotFound default />
</PassThroughPage>
Expand Down
40 changes: 28 additions & 12 deletions spotlight-client/src/SystemNarrativePage/SectionNavigation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.
// =============================================================================

import { Link, useParams } from "@reach/router";
import { format } from "d3-format";
import { rem } from "polished";
import React, { useEffect } from "react";
import { animated, useSpring } from "react-spring";
import styled from "styled-components/macro";
import { NAV_BAR_HEIGHT } from "../constants";
import getUrlForResource from "../routerUtils/getUrlForResource";
import normalizeRouteParams from "../routerUtils/normalizeRouteParams";
import { colors } from "../UiLibrary";
import Arrow from "../UiLibrary/Arrow";

Expand Down Expand Up @@ -105,12 +108,24 @@ const SectionNavigation: React.FC<NavigationProps> = ({
setActiveSection,
totalPages,
}) => {
const { tenantId, narrativeTypeId } = normalizeRouteParams(
useParams()
// these keys should always be present on this page
) as Required<
Pick<
ReturnType<typeof normalizeRouteParams>,
"tenantId" | "narrativeTypeId"
>
>;

const disablePrev = activeSection === 1;
const disableNext = activeSection === totalPages;
// TODO: how to have these still be useful links without direct URL interaction?
// go to top of the page instead of #1
const prevUrl = `#${activeSection > 2 ? activeSection - 1 : ""}`;
const nextUrl = `#${activeSection + 1}`;

// base is the current page, minus the section number
const urlBase = getUrlForResource({
page: "narrative",
params: { tenantId, narrativeTypeId },
});

return (
<SectionNav aria-label="page sections">
Expand All @@ -122,32 +137,33 @@ const SectionNavigation: React.FC<NavigationProps> = ({
<Arrow direction="up" faded />
</div>
) : (
<a
href={prevUrl}
onClick={(e) => {
e.preventDefault();
<Link
// the to props on these links is mainly for accessibility purposes, so they
// look like normal links. calling the setter is the critical step
to={`${urlBase}/${activeSection - 1}`}
onClick={() => {
setActiveSection(activeSection - 1);
}}
aria-label="previous section"
>
<Arrow direction="up" />
</a>
</Link>
)}
{disableNext ? (
<div>
<Arrow direction="down" faded />
</div>
) : (
<a
href={nextUrl}
<Link
to={`${urlBase}/${activeSection + 1}`}
onClick={(e) => {
e.preventDefault();
setActiveSection(activeSection + 1);
}}
aria-label="next section"
>
<Arrow direction="down" />
</a>
</Link>
)}
</SectionNav>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.
// =============================================================================

import { fireEvent, render, screen, within } from "@testing-library/react";
import { fireEvent, screen, waitFor, within } from "@testing-library/react";
import React from "react";
import contentFixture from "../contentModels/__fixtures__/tenant_content_exhaustive";
import { createMetricMapping } from "../contentModels/Metric";
Expand All @@ -25,11 +25,23 @@ import SystemNarrative, {
import SystemNarrativePage from ".";
import { SystemNarrativeContent } from "../contentApi/types";
import { MetricMapping } from "../contentModels/types";
import { renderWithRouter } from "../testUtils";

let allMetrics: MetricMapping;
let testNarrative: SystemNarrative;
let narrativeContent: SystemNarrativeContent;

jest.mock("@reach/router", () => {
return {
...jest.requireActual("@reach/router"),
// simulates being at the proper page without setting up a whole router
useParams: jest.fn().mockImplementation(() => ({
tenantId: "us_nd",
narrativeTypeId: "parole",
})),
};
});

beforeEach(() => {
allMetrics = createMetricMapping({
tenantId: "US_ND",
Expand All @@ -47,7 +59,7 @@ beforeEach(() => {
});

test("renders all the sections", () => {
render(<SystemNarrativePage narrative={testNarrative} />);
renderWithRouter(<SystemNarrativePage narrative={testNarrative} />);

expect(
screen.getByRole("heading", { name: narrativeContent.title, level: 1 })
Expand All @@ -63,7 +75,7 @@ test("renders all the sections", () => {
});

test("navigation", async () => {
render(<SystemNarrativePage narrative={testNarrative} />);
renderWithRouter(<SystemNarrativePage narrative={testNarrative} />);

const nextLabel = "next section";
const prevLabel = "previous section";
Expand All @@ -82,7 +94,9 @@ test("navigation", async () => {

// advance to section 2
fireEvent.click(nextLink);
expect(within(navRegion).getByText("02")).toBeInTheDocument();
await waitFor(() =>
expect(within(navRegion).getByText("02")).toBeInTheDocument()
);

const prevLink = screen.getByRole("link", { name: prevLabel });
expect(prevLink).toBeInTheDocument();
Expand Down Expand Up @@ -114,7 +128,7 @@ test("renders link tags in copy", async () => {
allMetrics,
});

render(<SystemNarrativePage narrative={testNarrative} />);
renderWithRouter(<SystemNarrativePage narrative={testNarrative} />);

expect(screen.getByRole("link", { name: "intro link" })).toBeInTheDocument();

Expand Down
58 changes: 51 additions & 7 deletions spotlight-client/src/SystemNarrativePage/SystemNarrativePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.
// =============================================================================

import { navigate, useParams } from "@reach/router";
import HTMLReactParser from "html-react-parser";
import { rem } from "polished";
import React, { useEffect, useRef, useState } from "react";
import React, { useCallback, useEffect, useRef, useState } from "react";
import { InView } from "react-intersection-observer";
import { config, useSpring } from "react-spring";
import { useSpring } from "react-spring";
import styled from "styled-components/macro";
import { NAV_BAR_HEIGHT } from "../constants";
import SystemNarrative from "../contentModels/SystemNarrative";
import getUrlForResource from "../routerUtils/getUrlForResource";
import normalizeRouteParams from "../routerUtils/normalizeRouteParams";
import { colors, typefaces } from "../UiLibrary";
import Arrow from "../UiLibrary/Arrow";
import { X_PADDING } from "./constants";
Expand Down Expand Up @@ -74,18 +77,39 @@ const SectionsContainer = styled.div``;
const SystemNarrativePage: React.FC<{
narrative: SystemNarrative;
}> = ({ narrative }) => {
const [activeSection, setActiveSection] = useState<number>(1);

const routeParams = useParams();
const sectionsContainerRef = useRef() as React.MutableRefObject<
HTMLDivElement
>;

// automated scrolling is a special case of section visibility;
// this flag lets us suspend in-page navigation actions while it is in progress
const [isScrolling, setIsScrolling] = useState(false);
const [activeSection, directlySetActiveSection] = useState(
// make sure we consume the section number in the URL, if any, on first mount
Number(routeParams.sectionNumber) || 1
);
// wrap the section state setter in a function that respects the flag
const setActiveSection = useCallback(
(sectionNumber: number) => {
if (!isScrolling) {
directlySetActiveSection(sectionNumber);
}
},
[isScrolling]
);
const [, setScrollSpring] = useSpring(() => ({
config: config.default,
onFrame: (props: { top: number }) => window.scrollTo({ top: props.top }),
// set the flag while animation is in progress
onRest: () => setIsScrolling(false),
onStart: () => setIsScrolling(true),
to: { top: window.scrollY },
}));

const { tenantId, narrativeTypeId } = normalizeRouteParams(routeParams);
// updating the active section has two key side effects:
// 1. smoothly scrolling to the active session
// 2. updating the page URL so the section can be linked to directly
useEffect(() => {
let scrollDestination;
// scroll to the corresponding section by calculating its offset
Expand All @@ -97,20 +121,40 @@ const SystemNarrativePage: React.FC<{
window.scrollY + desiredSection.getBoundingClientRect().top;
}

// in practice this should always be defined, this is just type safety
if (scrollDestination !== undefined) {
setScrollSpring({
to: { top: scrollDestination - NAV_BAR_HEIGHT },
from: { top: window.scrollY },
reset: true,
});

// these should always be defined on this page; more type safety
if (tenantId && narrativeTypeId) {
navigate(
`${getUrlForResource({
page: "narrative",
params: { tenantId, narrativeTypeId },
})}/${activeSection}`
);
}
}
}, [activeSection, sectionsContainerRef, setScrollSpring]);
}, [
activeSection,
narrativeTypeId,
sectionsContainerRef,
setScrollSpring,
tenantId,
]);

return (
<Container>
<SectionNavigation
activeSection={activeSection}
setActiveSection={setActiveSection}
// pagination UI should not respect the scrolling flag;
// in fact it should override it, otherwise the buttons
// will stop working while the animation is in progress
setActiveSection={directlySetActiveSection}
totalPages={narrative.sections.length + 1}
/>
<SectionsContainer ref={sectionsContainerRef}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.
// =============================================================================

import { LocationProvider } from "@reach/router";
import { render } from "@testing-library/react";
import React from "react";
import { autorun } from "mobx";
import waitForLocalhost from "wait-for-localhost";

Expand All @@ -31,3 +34,17 @@ export function reactImmediately(effect: () => void): void {
// and then immediately call the disposer to tear down the reaction
autorun(effect)();
}

const LocationContextWrapper: React.FC = ({ children }) => {
return <LocationProvider>{children}</LocationProvider>;
};

/**
* Convenience method for rendering components that use @reach/router hooks
* in a LocationContext to prevent rendering errors.
*/
export const renderWithRouter = (
ui: React.ReactElement
): ReturnType<typeof render> => {
return render(ui, { wrapper: LocationContextWrapper });
};

0 comments on commit 0bd1282

Please sign in to comment.