Skip to content

Commit

Permalink
Nudge the user to see the message on the 'My details' page
Browse files Browse the repository at this point in the history
This change adds a small notification dot above the link, a typical pattern showing there's something new on that page.

Refs #1396, ed9ec31
  • Loading branch information
thewilkybarkid committed Nov 20, 2023
1 parent ed9ec31 commit 4addf9f
Show file tree
Hide file tree
Showing 27 changed files with 182 additions and 53 deletions.
17 changes: 17 additions & 0 deletions assets/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -1937,7 +1937,24 @@ skip-link {
}

& :any-link {
position: relative;
color: inherit;

@media screen {
& > [role='status']:last-child {
position: absolute;
padding: 0.25em;
transform: translate(-50%, -50%);
inset-block-start: 0;
inset-inline-start: 100%;
border-radius: 50%;
background-color: var(--color-primary);

@media (forced-colors: active) {
background-color: LinkText;
}
}
}
}

@media screen {
Expand Down
9 changes: 9 additions & 0 deletions integration/log-in.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ test.extend(canLogIn).extend(areLoggedIn)('can view my details', async ({ javaSc
test.extend(canLogIn).extend(areLoggedIn).extend(isANewUser)(
'are prompted to view my details once',
async ({ page }) => {
await expect(page.getByRole('link', { name: 'My details' })).toContainText('New notification')

await page.mouse.move(0, 0)
await expect(page).toHaveScreenshot()

await page.getByRole('link', { name: 'My details' }).click()

await expect(page.getByRole('main')).toContainText('Welcome to PREreview!')
Expand All @@ -51,6 +56,10 @@ test.extend(canLogIn).extend(areLoggedIn).extend(isANewUser)(
await page.reload()

await expect(page.getByRole('main')).not.toContainText('Welcome to PREreview!')

await page.goto('/')

await expect(page.getByRole('link', { name: 'My details' })).not.toContainText('New notification')
},
)

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 14 additions & 1 deletion src/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as R from 'fp-ts/Reader'
import * as RA from 'fp-ts/ReadonlyArray'
import { flow, pipe } from 'fp-ts/function'
import * as s from 'fp-ts/string'
import { match } from 'ts-pattern'
import { type Html, type PlainText, html, rawHtml } from './html'
import * as assets from './manifest.json'
import {
Expand All @@ -25,6 +26,7 @@ import {
trainingsMatch,
} from './routes'
import type { User } from './user'
import type { UserOnboarding } from './user-onboarding'

export interface FathomEnv {
readonly fathomId?: string
Expand Down Expand Up @@ -59,6 +61,7 @@ export interface Page {
| 'trainings'
readonly js?: ReadonlyArray<Exclude<Assets<'.js'>, 'skip-link.js'>>
readonly user?: User
readonly userOnboarding?: UserOnboarding
}

export interface TemplatePageEnv {
Expand All @@ -75,6 +78,7 @@ export function page({
current,
js = [],
user,
userOnboarding,
}: Page): R.Reader<FathomEnv & PhaseEnv, Html> {
const scripts = pipe(js, RA.uniq(stringEq()), RA.concatW(skipLinks.length > 0 ? ['skip-link.js' as const] : []))

Expand Down Expand Up @@ -141,7 +145,16 @@ export function page({
<a
href="${format(myDetailsMatch.formatter, {})}"
${current === 'my-details' ? html`aria-current="page"` : ''}
>My details</a
>My
details${match(userOnboarding)
.with(
{ seenMyDetailsPage: false },
() =>
html` <span role="status"
><span class="visually-hidden">New notification</span></span
>`,
)
.otherwise(() => '')}</a
>
</li>`
: ''}
Expand Down
12 changes: 8 additions & 4 deletions src/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { deleteFlashMessage, getFlashMessage } from './flash-message'
import { type Html, html, sendHtml } from './html'
import type { Page, TemplatePageEnv } from './page'
import type { User } from './user'
import { type GetUserOnboardingEnv, maybeGetUserOnboarding } from './user-onboarding'

export type Response = PageResponse

Expand All @@ -34,7 +35,7 @@ export const handleResponse = (response: {
current?: Page['current']
response: Response
user?: User
}): RM.ReaderMiddleware<TemplatePageEnv, StatusOpen, ResponseEnded, never, void> =>
}): RM.ReaderMiddleware<GetUserOnboardingEnv & TemplatePageEnv, StatusOpen, ResponseEnded, never, void> =>
match(response)
.with({ response: { _tag: 'PageResponse' } }, handlePageResponse)
.exhaustive()
Expand All @@ -45,10 +46,12 @@ const handlePageResponse = ({
}: {
response: PageResponse
user?: User
}): RM.ReaderMiddleware<TemplatePageEnv, StatusOpen, ResponseEnded, never, void> =>
}): RM.ReaderMiddleware<GetUserOnboardingEnv & TemplatePageEnv, StatusOpen, ResponseEnded, never, void> =>
pipe(
RM.fromMiddleware(getFlashMessage(D.literal('logged-out', 'logged-in', 'blocked'))),
RM.chain(message =>
RM.of({}),
RM.apS('message', RM.fromMiddleware(getFlashMessage(D.literal('logged-out', 'logged-in', 'blocked')))),
RM.apS('userOnboarding', user ? RM.fromReaderTaskEither(maybeGetUserOnboarding(user.orcid)) : RM.of(undefined)),
RM.chainW(({ message, userOnboarding }) =>
RM.asks(({ templatePage }: TemplatePageEnv) =>
templatePage({
title: response.title,
Expand Down Expand Up @@ -94,6 +97,7 @@ const handlePageResponse = ({
current: response.current,
js: response.js.concat(...(message ? (['notification-banner.js'] as const) : [])),
user,
userOnboarding,
}),
),
),
Expand Down
1 change: 1 addition & 0 deletions src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ const router: P.Parser<RM.ReaderMiddleware<RouterEnv, StatusOpen, ResponseEnded,
),
env,
),
getUserOnboarding: withEnv(getUserOnboarding, env),
})),
),
),
Expand Down
6 changes: 6 additions & 0 deletions src/user-onboarding.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as RTE from 'fp-ts/ReaderTaskEither'
import type * as TE from 'fp-ts/TaskEither'
import { flow } from 'fp-ts/function'
import * as C from 'io-ts/Codec'
import type { Orcid } from 'orcid-id-ts'

Expand All @@ -24,6 +25,11 @@ export const getUserOnboarding = (
): RTE.ReaderTaskEither<GetUserOnboardingEnv, 'unavailable', UserOnboarding> =>
RTE.asksReaderTaskEither(RTE.fromTaskEitherK(({ getUserOnboarding }) => getUserOnboarding(orcid)))

export const maybeGetUserOnboarding = flow(
getUserOnboarding,
RTE.orElseW(() => RTE.of(undefined)),
)

export const saveUserOnboarding = (
orcid: Orcid,
userOnboarding: UserOnboarding,
Expand Down
105 changes: 77 additions & 28 deletions test/response.test.ts
Original file line number Diff line number Diff line change
@@ -1,53 +1,102 @@
import { test } from '@fast-check/jest'
import { describe, expect, jest } from '@jest/globals'
import * as E from 'fp-ts/Either'
import * as TE from 'fp-ts/TaskEither'
import { MediaType } from 'hyper-ts'
import { rawHtml } from '../src/html'
import type { TemplatePageEnv } from '../src/page'
import * as _ from '../src/response'
import type { GetUserOnboardingEnv } from '../src/user-onboarding'
import * as fc from './fc'
import { runMiddleware } from './middleware'
import { shouldNotBeCalled } from './should-not-be-called'

describe('handleResponse', () => {
describe('with a PageResponse', () => {
test.prop([fc.connection(), fc.pageResponse(), fc.option(fc.user(), { nil: undefined }), fc.html()])(
'templates the page',
async (connection, response, user, page) => {
const templatePage = jest.fn<TemplatePageEnv['templatePage']>(_ => page)

const actual = await runMiddleware(_.handleResponse({ response, user })({ templatePage }), connection)()

expect(actual).toStrictEqual(
E.right(
expect.arrayContaining([
{ type: 'setStatus', status: response.status },
{ type: 'setHeader', name: 'Content-Type', value: MediaType.textHTML },
{ type: 'setBody', body: page.toString() },
]),
),
)
expect(templatePage).toHaveBeenCalledWith({
title: response.title,
content: expect.stringContaining(response.main.toString()),
skipLinks: [[rawHtml('Skip to main content'), '#main']],
current: response.current,
js: response.js,
user,
})
},
)
describe('templates the page', () => {
test.prop([fc.connection(), fc.pageResponse(), fc.user(), fc.userOnboarding(), fc.html()])(
'when there is a user',
async (connection, response, user, userOnboarding, page) => {
const getUserOnboarding = jest.fn<GetUserOnboardingEnv['getUserOnboarding']>(_ => TE.right(userOnboarding))
const templatePage = jest.fn<TemplatePageEnv['templatePage']>(_ => page)

const actual = await runMiddleware(
_.handleResponse({ response, user })({
getUserOnboarding,
templatePage,
}),
connection,
)()

expect(actual).toStrictEqual(
E.right(
expect.arrayContaining([
{ type: 'setStatus', status: response.status },
{ type: 'setHeader', name: 'Content-Type', value: MediaType.textHTML },
{ type: 'setBody', body: page.toString() },
]),
),
)
expect(getUserOnboarding).toHaveBeenCalledWith(user.orcid)
expect(templatePage).toHaveBeenCalledWith({
title: response.title,
content: expect.stringContaining(response.main.toString()),
skipLinks: [[rawHtml('Skip to main content'), '#main']],
current: response.current,
js: response.js,
user,
userOnboarding,
})
},
)

test.prop([fc.connection(), fc.pageResponse(), fc.html()])(
"when there isn't a user",
async (connection, response, page) => {
const templatePage = jest.fn<TemplatePageEnv['templatePage']>(_ => page)

const actual = await runMiddleware(
_.handleResponse({
response,
user: undefined,
})({ getUserOnboarding: shouldNotBeCalled, templatePage }),
connection,
)()

expect(actual).toStrictEqual(
E.right(
expect.arrayContaining([
{ type: 'setStatus', status: response.status },
{ type: 'setHeader', name: 'Content-Type', value: MediaType.textHTML },
{ type: 'setBody', body: page.toString() },
]),
),
)
expect(templatePage).toHaveBeenCalledWith({
title: response.title,
content: expect.stringContaining(response.main.toString()),
skipLinks: [[rawHtml('Skip to main content'), '#main']],
current: response.current,
js: response.js,
user: undefined,
userOnboarding: undefined,
})
},
)
})

test.prop([
fc.connection(),
fc.pageResponse({ canonical: fc.lorem() }),
fc.option(fc.user(), { nil: undefined }),
fc.userOnboarding(),
fc.html(),
])('sets a canonical link', async (connection, response, user, page) => {
])('sets a canonical link', async (connection, response, user, userOnboarding, page) => {
const actual = await runMiddleware(
_.handleResponse({
response,
user,
})({ templatePage: () => page }),
})({ getUserOnboarding: () => TE.right(userOnboarding), templatePage: () => page }),
connection,
)()

Expand Down

0 comments on commit 4addf9f

Please sign in to comment.