Skip to content

Commit

Permalink
Remaining v2 polish (#375)
Browse files Browse the repository at this point in the history
  • Loading branch information
macfarlandian authored Mar 29, 2021
1 parent 1cc77e2 commit f6bb861
Show file tree
Hide file tree
Showing 28 changed files with 312 additions and 210 deletions.
2 changes: 2 additions & 0 deletions spotlight-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"@auth0/auth0-spa-js": "^1.13.1",
"@reach/router": "^1.3.4",
"@recidiviz/case-triage-components": "^0.2.2",
"@types/body-scroll-lock": "^2.6.1",
"@types/classnames": "^2.2.11",
"@types/d3-array": "^2.9.0",
"@types/d3-color": "^2.0.1",
Expand All @@ -42,6 +43,7 @@
"@types/topojson": "^3.2.2",
"@w11r/use-breakpoint": "^1.8.0",
"assert-never": "^1.2.1",
"body-scroll-lock": "^3.1.5",
"change-case": "^4.1.2",
"classnames": "^2.2.6",
"d3-array": "^2.12.0",
Expand Down
13 changes: 7 additions & 6 deletions spotlight-client/src/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
waitFor,
} from "@testing-library/react";
import testContent from "./contentApi/sources/us_nd";
import { NarrativesSlug } from "./routerUtils/types";
import { renderNavigableApp, segmentMock } from "./testUtils";

describe("navigation", () => {
Expand Down Expand Up @@ -70,7 +71,7 @@ describe("navigation", () => {

test("single narrative page", () => {
expect.hasAssertions();
const targetPath = "/us-nd/collections/prison";
const targetPath = `/us-nd/${NarrativesSlug}/prison`;
const lookupArgs = [
"heading",
{
Expand All @@ -84,7 +85,7 @@ describe("navigation", () => {

test("racial disparities narrative page", async () => {
expect.hasAssertions();
const targetPath = "/us-nd/collections/racial-disparities";
const targetPath = `/us-nd/${NarrativesSlug}/racial-disparities`;
const lookupArgs = [
"heading",
{
Expand Down Expand Up @@ -160,18 +161,18 @@ describe("navigation", () => {
expect(document.title).toBe("North Dakota — Spotlight by Recidiviz");
expect(segmentMock.page).toHaveBeenCalledTimes(1);

await act(() => navigate("/us-nd/collections/prison"));
await act(() => navigate(`/us-nd/${NarrativesSlug}/prison`));

expect(document.title).toBe(
"Prison — North Dakota — Spotlight by Recidiviz"
);
expect(segmentMock.page).toHaveBeenCalledTimes(2);

// in-page navigation doesn't trigger additional pageviews
await act(() => navigate("/us-nd/collections/prison/2"));
await act(() => navigate(`/us-nd/${NarrativesSlug}/prison/2`));
expect(segmentMock.page).toHaveBeenCalledTimes(2);

await act(() => navigate("/us-nd/collections/sentencing"));
await act(() => navigate(`/us-nd/${NarrativesSlug}/sentencing`));

expect(document.title).toBe(
"Sentencing — North Dakota — Spotlight by Recidiviz"
Expand Down Expand Up @@ -217,7 +218,7 @@ describe("navigation", () => {
});

test("invalid narrative", async () => {
renderNavigableApp({ route: "/us-nd/collections/invalid" });
renderNavigableApp({ route: `/us-nd/${NarrativesSlug}/invalid` });
expect(screen.getByRole(...notFoundRoleArgs)).toBeVisible();
expect(document.title).toBe(
"Page not found — North Dakota — Spotlight by Recidiviz"
Expand Down
4 changes: 1 addition & 3 deletions spotlight-client/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ const App: React.FC = () => {
<Redirect from="/us_nd/overview" to="/us-nd" noThrow />
<PassThroughPage path="/:tenantId">
<PageTenant path="/" />
<PageNarrative
path={`/${NarrativesSlug}/:narrativeTypeId/*sectionNumber`}
/>
<PageNarrative path={`/${NarrativesSlug}/:narrativeTypeId`} />
<PageNotFound path="/*" />
</PassThroughPage>
</Router>
Expand Down
6 changes: 3 additions & 3 deletions spotlight-client/src/NarrativeFooter/NarrativeFooter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,17 @@ const Footer: React.FC = () => {
if (!tenant) return null;

return (
<Container aria-label="collections">
<Container aria-label="data narratives">
<Heading>Continue Reading</Heading>
<OtherNarrativeLinks />
<Link
className="NarrativeFooter__BackLink"
to={getUrlForResource({
page: "narrative list",
page: "tenant",
params: { tenantId: tenant.id },
})}
>
<Arrow direction="left" /> Back to Collections
<Arrow direction="left" /> Back to Data Narratives
</Link>
</Container>
);
Expand Down
34 changes: 3 additions & 31 deletions spotlight-client/src/NarrativeLayout/NarrativeLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.
// =============================================================================

import { navigate, useParams } from "@reach/router";
import useBreakpoint from "@w11r/use-breakpoint";
import { rem } from "polished";
import React, { useCallback, useEffect, useRef, useState } from "react";
Expand All @@ -24,8 +23,6 @@ import { useSpring } from "react-spring/web.cjs";
import Sticker from "react-stickyfill";
import styled from "styled-components/macro";
import { NAV_BAR_HEIGHT } from "../constants";
import getUrlForResource from "../routerUtils/getUrlForResource";
import normalizeRouteParams from "../routerUtils/normalizeRouteParams";
import { X_PADDING } from "../SystemNarrativePage/constants";
import NarrativeNavigation from "./NarrativeNavigation";
import { LayoutSection } from "./types";
Expand Down Expand Up @@ -58,7 +55,6 @@ type NarrativeLayoutProps = {
};

const NarrativeLayout: React.FC<NarrativeLayoutProps> = ({ sections }) => {
const routeParams = useParams();
const sectionsContainerRef = useRef() as React.MutableRefObject<
HTMLDivElement
>;
Expand All @@ -73,10 +69,7 @@ const NarrativeLayout: React.FC<NarrativeLayoutProps> = ({ sections }) => {
isScrolling = false;
};

const [activeSection, directlySetActiveSection] = useState(
// make sure we consume the section number in the URL, if any, on first mount
Number(routeParams.sectionNumber) || 1
);
const [activeSection, directlySetActiveSection] = useState(1);
// wrap the section state setter in a function that respects the flag
const setActiveSection = useCallback(
(sectionNumber: number) => {
Expand All @@ -86,10 +79,6 @@ const NarrativeLayout: React.FC<NarrativeLayoutProps> = ({ sections }) => {
},
[isScrolling]
);
// keep section state in sync with URL if it changes externally (e.g. via nav link)
useEffect(() => {
directlySetActiveSection(Number(routeParams.sectionNumber) || 1);
}, [routeParams.sectionNumber]);

const [, setScrollSpring] = useSpring(() => ({
onFrame: (props: { top: number }) => {
Expand All @@ -110,7 +99,6 @@ const NarrativeLayout: React.FC<NarrativeLayoutProps> = ({ sections }) => {
to: { top: window.pageYOffset },
}));

const { tenantId, narrativeTypeId } = normalizeRouteParams(routeParams);
// updating the active section has two key side effects:
// 1. smoothly scrolling to the active section
// 2. updating the page URL so the section can be linked to directly
Expand All @@ -132,25 +120,8 @@ const NarrativeLayout: React.FC<NarrativeLayoutProps> = ({ sections }) => {
from: { top: window.pageYOffset },
reset: true,
});

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

return (
<Wrapper>
Expand All @@ -161,6 +132,7 @@ const NarrativeLayout: React.FC<NarrativeLayoutProps> = ({ sections }) => {
<NarrativeNavigation
activeSection={activeSection}
sections={sections}
setActiveSection={directlySetActiveSection}
/>
</NavStickyContainer>
</Sticker>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,29 @@
import { rem } from "polished";
import React, { useState } from "react";
import styled from "styled-components/macro";
import NavigationLink from "../../NavigationLink";
import { colors, Chevron } from "../../UiLibrary";

const StyledNavLink = styled(NavigationLink)`
const Button = styled.button.attrs({ type: "button" })`
background: none;
border: none;
cursor: ${(props) => (props.disabled ? "not-allowed" : "pointer")};
padding: ${rem(8)};
`;

type AdvanceLinkProps = {
activeSection: number;
disabled: boolean;
setActiveSection: (section: number) => void;
type: "previous" | "next";
urlBase: string;
};

const AdvanceLink: React.FC<AdvanceLinkProps> = ({
activeSection,
disabled,
setActiveSection,
type,
urlBase,
}) => {
let targetSection;
let targetSection: number;
let direction: "up" | "down";

if (type === "previous") {
Expand All @@ -54,18 +56,17 @@ const AdvanceLink: React.FC<AdvanceLinkProps> = ({
const color = hovered && !disabled ? colors.accent : undefined;

return (
<StyledNavLink
to={`${urlBase}/${targetSection}`}
<Button
onClick={() => !disabled && setActiveSection(targetSection)}
disabled={disabled}
replace
aria-label={`${type} section`}
onMouseOver={() => setHovered(true)}
onFocus={() => setHovered(true)}
onMouseOut={() => setHovered(false)}
onBlur={() => setHovered(false)}
>
<Chevron direction={direction} faded={disabled} color={color} />
</StyledNavLink>
</Button>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,14 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.
// =============================================================================

import { useParams } from "@reach/router";
import { format } from "d3-format";
import { rem } from "polished";
import React from "react";
import styled from "styled-components/macro";
import { DeepNonNullable } from "utility-types";
import getUrlForResource from "../../routerUtils/getUrlForResource";
import normalizeRouteParams from "../../routerUtils/normalizeRouteParams";
import { LayoutSection } from "../types";
import AdvanceLink from "./AdvanceLink";
import SectionLinks from "./SectionLinks";
import { SetSection } from "./types";

const formatPageNum = format("02");

Expand All @@ -52,27 +49,14 @@ const SectionNumberFaded = styled(SectionNumber)`
type NavigationProps = {
activeSection: number;
sections: LayoutSection[];
setActiveSection: SetSection;
};

const SectionNavigation: React.FC<NavigationProps> = ({
activeSection,
sections,
setActiveSection,
}) => {
const { tenantId, narrativeTypeId } = normalizeRouteParams(
useParams()
// these keys should always be present on this page
) as DeepNonNullable<
Pick<
ReturnType<typeof normalizeRouteParams>,
"tenantId" | "narrativeTypeId"
>
>;
// base is the current page, minus the section number
const urlBase = getUrlForResource({
page: "narrative",
params: { tenantId, narrativeTypeId },
});

const totalPages = sections.length;

// these will be used to toggle prev/next links
Expand All @@ -83,17 +67,17 @@ const SectionNavigation: React.FC<NavigationProps> = ({
<SectionNav aria-label="page sections">
<SectionNumber>{formatPageNum(activeSection)}</SectionNumber>
<SectionNumberFaded>{formatPageNum(totalPages)}</SectionNumberFaded>
<SectionLinks {...{ activeSection, sections, urlBase }} />
<SectionLinks {...{ activeSection, sections, setActiveSection }} />
<AdvanceLink
urlBase={urlBase}
activeSection={activeSection}
disabled={disablePrev}
setActiveSection={setActiveSection}
type="previous"
/>
<AdvanceLink
urlBase={urlBase}
activeSection={activeSection}
disabled={disableNext}
setActiveSection={setActiveSection}
type="next"
/>
</SectionNav>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.
// =============================================================================

import { Link } from "@reach/router";
import { rem } from "polished";
import React, { useEffect } from "react";
import { animated, useSpring, useSprings } from "react-spring/web.cjs";
import styled from "styled-components/macro";
import { colors } from "../../UiLibrary";
import { LayoutSection } from "../types";
import { SetSection } from "./types";
import { THUMB_SIZE } from "./utils";

const PageProgressContainer = styled.div`
Expand Down Expand Up @@ -64,13 +64,17 @@ const SectionList = styled.ul`

const SectionListItem = styled.li``;

const SectionLink = styled(Link)`
const SectionLink = styled.button.attrs({ type: "button" })`
border: none;
background: none;
color: ${colors.text};
cursor: pointer;
display: flex;
height: ${rem(THUMB_SIZE.height)};
justify-content: center;
margin-bottom: ${rem(THUMB_SIZE.paddingBottom)};
position: relative;
text-align: left;
width: 100%;
`;

Expand Down Expand Up @@ -100,8 +104,8 @@ const getThumbOffset = (activeSection: number) =>
const SectionLinks: React.FC<{
activeSection: number;
sections: LayoutSection[];
urlBase: string;
}> = ({ activeSection, sections, urlBase }) => {
setActiveSection: SetSection;
}> = ({ activeSection, sections, setActiveSection }) => {
const totalPages = sections.length;

const progressBarHeight =
Expand Down Expand Up @@ -169,8 +173,8 @@ const SectionLinks: React.FC<{
return (
<SectionListItem key={section.title}>
<SectionLink
to={`${urlBase}/${index + 1}`}
replace
// sections are 1-indexed for display purposes
onClick={() => setActiveSection(index + 1)}
onMouseOver={showLinkLabel(index)}
onFocus={showLinkLabel(index)}
onMouseOut={hideLinkLabel(index)}
Expand Down
18 changes: 18 additions & 0 deletions spotlight-client/src/NarrativeLayout/NarrativeNavigation/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Recidiviz - a data platform for criminal justice reform
// Copyright (C) 2021 Recidiviz, Inc.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
// =============================================================================

export type SetSection = (section: number) => void;
Loading

0 comments on commit f6bb861

Please sign in to comment.