Skip to content

Commit

Permalink
bugfix: h1 on each page doesn't update when changing pages (#121)
Browse files Browse the repository at this point in the history
* Fix New Submission heading, Dashboard on login still broken

* progress

* Working for New Submission, add tests

* Tests passing
  • Loading branch information
haworku committed May 3, 2021
1 parent 3346b13 commit 6959c89
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 47 deletions.
2 changes: 1 addition & 1 deletion services/app-web/src/components/Header/Header.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ describe('Header', () => {
})
await waitFor(() =>
expect(screen.getByRole('heading')).toHaveTextContent(
'Minnesota New submission'
/Minnesota/
)
)
})
Expand Down
22 changes: 4 additions & 18 deletions services/app-web/src/components/Header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ import { NavLink } from 'react-router-dom'
import { idmRedirectURL } from '../../pages/Auth/cognitoAuth'
import onemacLogo from '../../assets/images/onemac-logo.svg'
import styles from './Header.module.scss'

import { getRouteName } from '../../constants/routes'
import { useHistory } from 'react-router-dom'
import { useAuth } from '../../contexts/AuthContext'
import { usePage } from '../../contexts/PageContext'
import { AuthModeType } from '../../common-code/domain-models'
import { Logo } from '../Logo/Logo'
import { PageHeadingRow } from './PageHeadingRow'
import { User } from '../../gen/gqlClient'
import { PageHeadingsRecord, getRouteName } from '../../constants/routes'

export type HeaderProps = {
authMode: AuthModeType
Expand All @@ -33,23 +34,8 @@ export const Header = ({
}: HeaderProps): React.ReactElement => {
const { logout, loggedInUser, loginStatus } = useAuth()
const history = useHistory()
const routeName = getRouteName(history.location.pathname)
const { heading } = usePage()

/*
Dynamically calculate heading in priority order
1. If there a constant heading set up, use that
2. Otherwise, use whatever is in the PageContext
3. Fallback in case of new route
*/

const pageHeadingText =
routeName !== 'UNKNOWN_ROUTE' &&
Object.prototype.hasOwnProperty.call(PageHeadingsRecord, routeName)
? PageHeadingsRecord[routeName]
: heading
? heading
: '' // fallback for safety
const route = getRouteName(history.location.pathname)

const handleLogout = (
e: React.MouseEvent<HTMLButtonElement, MouseEvent>
Expand Down Expand Up @@ -134,7 +120,7 @@ export const Header = ({
</GridContainer>
</div>
<PageHeadingRow
heading={pageHeadingText}
heading={route !== 'UNKNOWN_ROUTE' ? heading : undefined}
isLoading={loginStatus === 'LOADING'}
loggedInUser={loggedInUser}
/>
Expand Down
10 changes: 6 additions & 4 deletions services/app-web/src/components/Header/PageHeadingRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type PageHeadingProps = {

export const PageHeadingRow = ({
isLoading = false,
heading = 'Dashboard',
heading,
loggedInUser,
}: PageHeadingProps): React.ReactElement => {
return loggedInUser ? (
Expand All @@ -32,9 +32,11 @@ export const PageHeadingRow = ({
</div>
<PageHeading>
<span>{loggedInUser.state.name}&nbsp;</span>
<span className="font-heading-lg text-light">
{heading}
</span>
{heading && (
<span className="font-heading-lg text-light">
{heading}
</span>
)}
</PageHeading>
</Grid>
</GridContainer>
Expand Down
6 changes: 4 additions & 2 deletions services/app-web/src/constants/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,16 @@ const RoutesRecord: Record<RouteT, string> = {
SUBMISSIONS_REVIEW_SUBMIT: '/submissions/:id/review-and-submit',
}

// Static page headings used in <header> h1. Dynamic headings, when necessary, are set in page specific parent component.
// Static page headings used in <header> h1 when logged in. Dynamic headings, when necessary, are set in page specific parent component.
const PageHeadingsRecord: Record<string, string> = {
ROOT: 'Dashboard',
DASHBOARD: 'Dashboard',
SUBMISSIONS_NEW: 'New submission',
}

// Static page titles used in <title>.
// Every route must have a fallback page title. Dynamic page title logic are set in AppRoutes.tsx
const PageTitlesRecord: Record<RouteT, string> = {
const PageTitlesRecord: Record<RouteT | 'UNKNOWN_ROUTE', string> = {
ROOT: 'Home - Managed Care Review',
AUTH: 'Login - Managed Care Review',
HELP: 'Help - Managed Care Review',
Expand All @@ -62,6 +63,7 @@ const PageTitlesRecord: Record<RouteT, string> = {
SUBMISSIONS_CONTACTS: 'Contacts - Managed Care Review',
SUBMISSIONS_DOCUMENTS: 'Documents - Managed Care Review',
SUBMISSIONS_REVIEW_SUBMIT: 'Review and Submit - Managed Care Review',
UNKNOWN_ROUTE: 'Not Found - Managed Care Review'
}

const getRouteName = (pathname: string): RouteT | 'UNKNOWN_ROUTE' => {
Expand Down
30 changes: 26 additions & 4 deletions services/app-web/src/contexts/PageContext.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import * as React from 'react'
import { PageHeadingsRecord, getRouteName } from '../constants/routes'

/*
Stores page specific context that must be shared across the application
such as the presence of a dynamic page heading
Use sparingly.
Intended for page specific context that must be shared across the application, not for data that can be fetched from the api.
*/
type PageContextType = {
heading?: string
updateHeading: (headingText?: string) => void
updateHeading: (pathname: string, headingText?: string) => void
}

const PageContext = React.createContext((null as unknown) as PageContextType)
Expand All @@ -17,7 +18,28 @@ export type PageProviderProps = {
const PageProvider: React.FC = ({ children }) => {
const [heading, setHeading] = React.useState<string | undefined>(undefined)

const updateHeading = (text?: string) => setHeading(text)
/*
Set headings in priority order
1. If there a custom heading, use that
2. Otherwise, use default headings based on the PageHeadingsRecord
3. Do not show any page-specific headings when user is logged out (handled in Header component)
4. Do not show any headings when logged in user is on a Not Found page (handled in Header component)
*/
const updateHeading = (pathname: string, customHeading?: string) => {
const routeName = getRouteName(pathname)
const defaultHeading = Object.prototype.hasOwnProperty.call(
PageHeadingsRecord,
routeName
)
? PageHeadingsRecord[routeName]
: undefined

setHeading((prev) => {
if (!defaultHeading && !customHeading) return prev
return customHeading ? customHeading : defaultHeading
})
}

return (
<PageContext.Provider
value={{ heading, updateHeading }}
Expand Down
13 changes: 8 additions & 5 deletions services/app-web/src/pages/App/AppRoutes.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react'
import React, { useEffect } from 'react'
import { Switch, Route } from 'react-router-dom'
import { useLocation } from 'react-router'

Expand All @@ -14,7 +14,7 @@ import { NewStateSubmissionForm } from '../StateSubmissionForm/NewStateSubmissio
import { SubmissionDescriptionExamples } from '../Help/SubmissionDescriptionExamples'
import { useTitle } from '../../hooks/useTitle'
import { getRouteName, PageTitlesRecord } from '../../constants/routes'

import { usePage } from '../../contexts/PageContext'
import { AuthModeType, assertNever } from '../../common-code/domain-models'

function componentForAuthMode(
Expand All @@ -40,18 +40,21 @@ export const AppRoutes = ({
const { loggedInUser } = useAuth()
const { pathname } = useLocation()
const route = getRouteName(pathname)
const { updateHeading } = usePage()

/*
Add page titles throughout the application
Add page titles and headings throughout the application
*/
const title =
route === 'ROOT' && loggedInUser
? PageTitlesRecord['DASHBOARD']
: route === 'UNKNOWN_ROUTE'
? 'Managed Care Review'
: PageTitlesRecord[route]
useTitle(title)

useEffect(() => {
updateHeading(pathname)
}, [pathname, updateHeading])

const authComponent = componentForAuthMode(authMode)

const AuthenticatedRoutes = (): React.ReactElement => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useEffect } from 'react'

import { GridContainer } from '@trussworks/react-uswds'
import { Switch, Route, useParams } from 'react-router-dom'
import { Switch, Route, useParams, useLocation } from 'react-router-dom'

import { Error404 } from '../Errors/Error404'
import { GenericError } from '../Errors/GenericError'
Expand All @@ -17,24 +17,21 @@ import { useFetchDraftSubmissionQuery } from '../../gen/gqlClient'

export const StateSubmissionForm = (): React.ReactElement => {
const { id } = useParams<{ id: string }>()

const { pathname } = useLocation()
const { updateHeading } = usePage()

// const routeName = getRouteName(pathname)
const { data, loading, error } = useFetchDraftSubmissionQuery({
variables: {
input: {
submissionID: id,
},
},
})
const draft = data?.fetchDraftSubmission?.draftSubmission

useEffect(() => {
// We have to updateHeading inside useEffect so that we don't update two components at the same time
const draft = data?.fetchDraftSubmission?.draftSubmission
if (draft) {
updateHeading(draft.name)
}
}, [data, updateHeading])
updateHeading(pathname, draft?.name)
}, [updateHeading, pathname, draft])

if (loading) {
return (
Expand All @@ -49,13 +46,10 @@ export const StateSubmissionForm = (): React.ReactElement => {
return <GenericError />
}

const draft = data?.fetchDraftSubmission?.draftSubmission

if (draft === undefined || draft === null) {
console.log('got undefined back from loaded showDraftSubmission')
return <Error404 />
}

return (
<GridContainer>
<Switch>
Expand Down
4 changes: 3 additions & 1 deletion tests/cypress/integration/stateSubmission.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ describe('State Submission', () => {
cy.login()
cy.visit('/submissions/not-a-draft-submission/type')
cy.findByText('404 / Page not found').should('exist')
cy.findByText('Dashboard').not('exist')
})

it('user can start a new submission and continue with valid input', () => {
Expand All @@ -13,7 +14,7 @@ describe('State Submission', () => {
force: true,
})
cy.location('pathname').should('eq', '/submissions/new')

cy.findByText('New submission').should('exist')
// Fill out some fields but not all
cy.findByLabelText('Contract action only').safeClick()
cy.findByRole('combobox', { name: 'Program' }).select('msho')
Expand All @@ -40,6 +41,7 @@ describe('State Submission', () => {
name: 'Continue',
}).safeClick()
cy.findByText('Contract details').should('exist')
cy.findByText(/MN-MSHO-/).should('exist')
})

it('user can edit a draft submission type', () => {
Expand Down
2 changes: 2 additions & 0 deletions tests/cypress/support/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ Cypress.Commands.add('login', () => {
cy.visit('/')
cy.findByText('Sign In').click()
const authMode = Cypress.env('AUTH_MODE')
console.log(authMode, 'authmode')
if (authMode === 'LOCAL') {
cy.findByTestId('AangButton').click()
} else if (authMode === 'AWS_COGNITO') {
Expand All @@ -18,4 +19,5 @@ Cypress.Commands.add('login', () => {
}
// this login/initial fetch can take a little while.
cy.url({ timeout: 10_000 }).should('match', /.*dashboard$/)
cy.findByRole('heading', {level: 1, name: /Dashboard/ })
})

0 comments on commit 6959c89

Please sign in to comment.