From 3758ae8ed85c8d6706e449b8a7cfcdcaf954715f Mon Sep 17 00:00:00 2001 From: David Evans Date: Mon, 2 Dec 2024 22:53:42 +0000 Subject: [PATCH] Improve front page --- e2e/pages/RetroCreate.ts | 8 +- e2e/pages/RetroList.ts | 47 --- e2e/pages/Welcome.ts | 64 ++- e2e/tests/basicflow.test.ts | 24 +- frontend/src/components/App.less | 5 - frontend/src/components/App.test.tsx | 5 +- frontend/src/components/App.tsx | 17 +- frontend/src/components/Footer.tsx | 5 +- frontend/src/components/RetroRouter.tsx | 4 +- .../archive-list/ArchiveListPage.tsx | 14 +- .../src/components/archive/ArchivePage.tsx | 36 +- frontend/src/components/colours.less | 4 +- .../src/components/common/Loader.test.tsx | 36 +- frontend/src/components/common/Loader.tsx | 35 +- frontend/src/components/form.less | 85 ++-- .../src/components/hocs/withUserToken.tsx | 22 - .../components/not-found/NotFoundPage.less | 5 + .../src/components/password/PasswordPage.less | 4 +- .../components/password/PasswordPage.test.tsx | 5 +- .../retro-create/RetroCreatePage.less | 2 - .../retro-create/RetroCreatePage.tsx | 37 +- .../src/components/retro-create/RetroForm.tsx | 377 +++++++++--------- .../retro-create/RetroImportPage.less | 2 - .../retro-create/RetroImportPage.tsx | 27 -- .../components/retro-create/SlugEntry.less | 46 +++ .../src/components/retro-create/SlugEntry.tsx | 118 +++--- .../src/components/retro-list/RetroList.tsx | 23 -- .../components/retro-list/RetroListPage.less | 27 -- .../retro-list/RetroListPage.test.tsx | 29 -- .../components/retro-list/RetroListPage.tsx | 28 -- .../retro-not-found/RetroNotFoundPage.tsx | 45 +++ .../retro-settings/RetroSettingsPage.test.tsx | 14 +- .../retro-settings/SettingsForm.tsx | 8 +- .../RetroLink.test.tsx | 0 .../{retro-list => welcome}/RetroLink.tsx | 0 .../RetroList.test.tsx | 21 +- frontend/src/components/welcome/RetroList.tsx | 17 + .../welcome/RetroNavigationForm.tsx | 38 ++ .../src/components/welcome/WelcomePage.less | 60 ++- .../components/welcome/WelcomePage.test.tsx | 98 ++--- .../src/components/welcome/WelcomePage.tsx | 108 +++-- .../src/hooks/data/useSlugAvailability.ts | 56 +++ frontend/src/integration.test.tsx | 32 +- 43 files changed, 814 insertions(+), 824 deletions(-) delete mode 100644 e2e/pages/RetroList.ts delete mode 100644 frontend/src/components/hocs/withUserToken.tsx delete mode 100644 frontend/src/components/retro-create/RetroCreatePage.less delete mode 100644 frontend/src/components/retro-create/RetroImportPage.less delete mode 100644 frontend/src/components/retro-create/RetroImportPage.tsx create mode 100644 frontend/src/components/retro-create/SlugEntry.less delete mode 100644 frontend/src/components/retro-list/RetroList.tsx delete mode 100644 frontend/src/components/retro-list/RetroListPage.less delete mode 100644 frontend/src/components/retro-list/RetroListPage.test.tsx delete mode 100644 frontend/src/components/retro-list/RetroListPage.tsx create mode 100644 frontend/src/components/retro-not-found/RetroNotFoundPage.tsx rename frontend/src/components/{retro-list => welcome}/RetroLink.test.tsx (100%) rename frontend/src/components/{retro-list => welcome}/RetroLink.tsx (100%) rename frontend/src/components/{retro-list => welcome}/RetroList.test.tsx (52%) create mode 100644 frontend/src/components/welcome/RetroList.tsx create mode 100644 frontend/src/components/welcome/RetroNavigationForm.tsx create mode 100644 frontend/src/hooks/data/useSlugAvailability.ts diff --git a/e2e/pages/RetroCreate.ts b/e2e/pages/RetroCreate.ts index 50e33168..ceb22251 100644 --- a/e2e/pages/RetroCreate.ts +++ b/e2e/pages/RetroCreate.ts @@ -1,7 +1,7 @@ import { By, WebDriver } from 'selenium-webdriver'; import { Page } from './common/Page'; import { Retro } from './Retro'; -import { RetroList } from './RetroList'; +import { Welcome } from './Welcome'; export class RetroCreate extends Page { private slug?: string; @@ -10,10 +10,10 @@ export class RetroCreate extends Page { super(driver, '/create', '.page-retro-create'); } - public async clickListRetros() { - await this.click(By.linkText('My Retros')); + public async clickAccount() { + await this.click(By.linkText('Account')); - return new RetroList(this.driver).wait(); + return new Welcome(this.driver).wait(); } public setName(name: string) { diff --git a/e2e/pages/RetroList.ts b/e2e/pages/RetroList.ts deleted file mode 100644 index 65ddc7cf..00000000 --- a/e2e/pages/RetroList.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { By, WebDriver } from 'selenium-webdriver'; -import { Page } from './common/Page'; -import { RetroCreate } from './RetroCreate'; -import { Retro } from './Retro'; - -export class RetroList extends Page { - public constructor(driver: WebDriver) { - super(driver, '/', '.page-retro-list'); - } - - public async clickCreateRetro() { - await this.click(By.linkText('Create Retro')); - - return new RetroCreate(this.driver).wait(); - } - - public async getRetroNames(): Promise { - const items = await this.getRetroItems(); - return Promise.all(items.map((item) => item.getText())); - } - - public async clickRetroNamed(name: string) { - const names = await this.getRetroNames(); - const index = names.indexOf(name); - if (index === -1) { - throw new Error(`No retro named ${name}`); - } - - const item = await this.getRetroItemAtIndex(index); - await item.click(); - - return new Retro(this.driver, 'unknown').wait(); - } - - private getRetroItems() { - return this.findElements(By.css('.retro-link')); - } - - private async getRetroItemAtIndex(index: number) { - const items = await this.getRetroItems(); - const item = items[index]; - if (!item) { - throw new Error(`No retro item at index ${index}`); - } - return item; - } -} diff --git a/e2e/pages/Welcome.ts b/e2e/pages/Welcome.ts index b89c9249..66dca6ac 100644 --- a/e2e/pages/Welcome.ts +++ b/e2e/pages/Welcome.ts @@ -3,7 +3,7 @@ import { Page } from './common/Page'; import { RetroCreate } from './RetroCreate'; import { SsoLogin } from './SsoLogin'; import { Security } from './Security'; -import { RetroList } from './RetroList'; +import { Retro } from './Retro'; export class Welcome extends Page { public constructor(driver: WebDriver) { @@ -14,31 +14,53 @@ export class Welcome extends Page { return this.getHeader().getText(); } - public async clickCreateRetro() { - await this.click(By.css('.link-create')); + public async getRetroNames(): Promise { + const items = await this.getRetroItems(); + return Promise.all(items.map((item) => item.getText())); + } - return new RetroCreate(this.driver).wait(); + public async clickRetroNamed(name: string) { + const names = await this.getRetroNames(); + const index = names.indexOf(name); + if (index === -1) { + throw new Error(`No retro named ${name}`); + } + + const item = await this.getRetroItemAtIndex(index); + await item.click(); + + return new Retro(this.driver, 'unknown').wait(); } - public async clickListRetros() { - await this.click(By.linkText('My Retros')); + public async clickCreateRetro() { + await this.click(By.css('.link-create')); - return new RetroList(this.driver).wait(); + return new RetroCreate(this.driver).wait(); } - public async clickLoginWithGoogle() { + public async clickLoginWithGoogle( + expectation: K, + ): Promise>> { await this.click(By.css('.sso-google')); - return new SsoLogin(this.driver, RetroCreate).wait(); + return new SsoLogin( + this.driver, + LoginTargets[expectation] as any, + ).wait() as any; } - public async loginAs(userName: string) { - const ssoLogin = await this.clickLoginWithGoogle(); + public async loginAs( + userName: string, + expectation: K, + ): Promise> { + const ssoLogin = await this.clickLoginWithGoogle(expectation); return await ssoLogin.loginAs(userName); } public async clickSecurity() { - const element = this.findElement(By.css('.link-security')); + const element = this.findElement( + By.linkText('Privacy & Security information'), + ); // avoid opening a new tab, as this is difficult to manage await this.driver.executeScript( 'arguments[0].setAttribute("target", "_self");', @@ -49,7 +71,25 @@ export class Welcome extends Page { return new Security(this.driver).wait(); } + private getRetroItems() { + return this.findElements(By.css('.retro-link')); + } + + private async getRetroItemAtIndex(index: number) { + const items = await this.getRetroItems(); + const item = items[index]; + if (!item) { + throw new Error(`No retro item at index ${index}`); + } + return item; + } + private getHeader() { return this.findElement(By.css('h1')); } } + +const LoginTargets = { + hasRetros: Welcome, + noRetros: RetroCreate, +}; diff --git a/e2e/tests/basicflow.test.ts b/e2e/tests/basicflow.test.ts index fc047167..196cf77f 100644 --- a/e2e/tests/basicflow.test.ts +++ b/e2e/tests/basicflow.test.ts @@ -3,7 +3,6 @@ import { getDownloadedBytes, Mbps } from '../helpers/downloadProfiler'; import { type Welcome } from '../pages/Welcome'; import { type Password } from '../pages/Password'; import { type RetroCreate } from '../pages/RetroCreate'; -import { type RetroList } from '../pages/RetroList'; import { type Retro } from '../pages/Retro'; import { type RetroArchiveList } from '../pages/RetroArchiveList'; import { type RetroArchive } from '../pages/RetroArchive'; @@ -53,7 +52,7 @@ describe('Refacto', { stopAtFirstFailure: true, timeout }, () => { }); it('triggers a login flow when requested', async () => { - const ssoLogin = await welcome.clickLoginWithGoogle(); + const ssoLogin = await welcome.clickLoginWithGoogle('noRetros'); await ssoLogin.setIdentifier(userName); create = await ssoLogin.submit(); }); @@ -207,29 +206,28 @@ describe('Refacto', { stopAtFirstFailure: true, timeout }, () => { describe('retro list', { stopAtFirstFailure: true }, () => { const retroName = 'My Retro Renamed'; - let retroList: RetroList; + let welcome: Welcome; it('prompts to log in when loaded', async () => { - const welcome = await user1.navigateToWelcome(); - const retroCreate = await welcome.loginAs(userName); - retroList = await retroCreate.clickListRetros(); + welcome = await user1.navigateToWelcome(); + welcome = await welcome.loginAs(userName, 'hasRetros'); }); it('displays retros created by the current user', async () => { - expect(await retroList.getRetroNames()).toEqual([retroName]); + expect(await welcome.getRetroNames()).toEqual([retroName]); }); it('loads linked retros without needing a password', async () => { - retro = await retroList.clickRetroNamed(retroName); + retro = await welcome.clickRetroNamed(retroName); expect(await retro.getActionItemLabels()).toEqual(['another action']); }); - it('does not list retros from other users', async () => { - const welcome = await user1.navigateToWelcome(); - const retroCreate = await welcome.loginAs('nobody'); - retroList = await retroCreate.clickListRetros(); + it('navigates directly to retro creation if user has no retros', async () => { + welcome = await user1.navigateToWelcome(); + const retroCreate = await welcome.loginAs('nobody', 'noRetros'); - expect(await retroList.getRetroNames()).toEqual([]); + welcome = await retroCreate.clickAccount(); + expect(await welcome.getRetroNames()).toEqual([]); }); }); diff --git a/frontend/src/components/App.less b/frontend/src/components/App.less index ed7638ab..6b7c65eb 100644 --- a/frontend/src/components/App.less +++ b/frontend/src/components/App.less @@ -14,11 +14,6 @@ button { article { background: @beige-back; border-bottom: 1px solid #ffffff; /* prevent background clipping */ - - > p { - margin: 64px 32px; - text-align: center; - } } a:link, diff --git a/frontend/src/components/App.test.tsx b/frontend/src/components/App.test.tsx index 1dac3570..728eabba 100644 --- a/frontend/src/components/App.test.tsx +++ b/frontend/src/components/App.test.tsx @@ -1,15 +1,16 @@ import { Router } from 'wouter'; import staticLocationHook from 'wouter/static-location'; -import { render } from 'flexible-testing-library-react'; +import { act, render } from 'flexible-testing-library-react'; import { App } from './App'; describe('App', () => { - it('renders without error', () => { + it('renders without error', async () => { render( , ); + await act(() => Promise.resolve()); // slug existence update }); }); diff --git a/frontend/src/components/App.tsx b/frontend/src/components/App.tsx index 74cdf4f3..562b724e 100644 --- a/frontend/src/components/App.tsx +++ b/frontend/src/components/App.tsx @@ -1,4 +1,4 @@ -import type { ComponentType, FC, ReactNode } from 'react'; +import type { FC, ReactNode } from 'react'; import { Route, Switch } from 'wouter'; import { RedirectRoute } from './RedirectRoute'; import { Footer } from './Footer'; @@ -7,10 +7,7 @@ import { RetroRouter } from './RetroRouter'; import { WelcomePage } from './welcome/WelcomePage'; import { SecurityPage } from './security/SecurityPage'; import { RetroCreatePage } from './retro-create/RetroCreatePage'; -import { RetroImportPage } from './retro-create/RetroImportPage'; -import { RetroListPage } from './retro-list/RetroListPage'; import { NotFoundPage } from './not-found/NotFoundPage'; -import { useUserToken } from '../hooks/data/useUserToken'; import './App.less'; export const App: FC = () => ( @@ -22,7 +19,7 @@ export const App: FC = () => ( )} - + @@ -31,7 +28,7 @@ export const App: FC = () => ( - + {({ slug = '' }) => } @@ -48,11 +45,3 @@ export const App: FC = () => (
); - -const SwitchLoggedIn: FC<{ - yes: ComponentType<{ userToken: string }>; - no: ComponentType; -}> = ({ yes: Yes, no: No }) => { - const [userToken] = useUserToken(); - return userToken ? : ; -}; diff --git a/frontend/src/components/Footer.tsx b/frontend/src/components/Footer.tsx index b6e7fc09..f245a4f1 100644 --- a/frontend/src/components/Footer.tsx +++ b/frontend/src/components/Footer.tsx @@ -19,6 +19,9 @@ export const Footer = memo(() => ( > GitLab - ) + {') - '} + + Privacy & Security information +
)); diff --git a/frontend/src/components/RetroRouter.tsx b/frontend/src/components/RetroRouter.tsx index d4d97667..1f7e8aea 100644 --- a/frontend/src/components/RetroRouter.tsx +++ b/frontend/src/components/RetroRouter.tsx @@ -3,7 +3,7 @@ import { Route, Switch, useLocation } from 'wouter'; import { type Retro } from '../shared/api-entities'; import { type RetroDispatch } from '../api/RetroTracker'; import { retroTracker, slugTracker } from '../api/api'; -import { RetroCreatePage } from './retro-create/RetroCreatePage'; +import { RetroNotFoundPage } from './retro-not-found/RetroNotFoundPage'; import { PasswordPage } from './password/PasswordPage'; import { RetroPage } from './retro/RetroPage'; import { ArchiveListPage } from './archive-list/ArchiveListPage'; @@ -93,7 +93,7 @@ export const RetroRouter: FC = ({ slug }) => { ); if (slugError === 'not found') { - return ; + return ; } const error = slugError || retroTokenError; diff --git a/frontend/src/components/archive-list/ArchiveListPage.tsx b/frontend/src/components/archive-list/ArchiveListPage.tsx index e2d4591b..7fc05a7f 100644 --- a/frontend/src/components/archive-list/ArchiveListPage.tsx +++ b/frontend/src/components/archive-list/ArchiveListPage.tsx @@ -1,7 +1,7 @@ import { memo } from 'react'; import { type RetroPagePropsT } from '../RetroRouter'; import { Header } from '../common/Header'; -import { Loader } from '../common/Loader'; +import { LoadingError, LoadingIndicator } from '../common/Loader'; import { ApiDownload } from '../common/ApiDownload'; import { useArchiveList } from '../../hooks/data/useArchiveList'; import { ArchiveList } from './ArchiveList'; @@ -22,11 +22,13 @@ export const ArchiveListPage = memo(({ retroToken, retro }: PropsT) => { action: `/retros/${encodeURIComponent(retro.slug)}`, }} /> - + {archivesError ? ( + + ) : !archives ? ( + + ) : ( + + )}
- undefined, - } - : null - } - /> + {archiveError ? ( + + ) : !archive ? ( + + ) : ( + undefined} + /> + )} ); }, diff --git a/frontend/src/components/colours.less b/frontend/src/components/colours.less index 8f62e4ed..d70e3bd3 100644 --- a/frontend/src/components/colours.less +++ b/frontend/src/components/colours.less @@ -12,8 +12,8 @@ @beige-back: hsl(45deg, 20%, 92%); @beige-shadow: hsl(45deg, 20%, 80%); -@green-button-back: #50e3c2; -@green-button-dark: @green-back; +@green-button-back: hsl(172deg, 72%, 60%); +@green-button-dark: hsl(172deg, 55%, 50%); @green-icon: @green-back; @red-icon: #aa3300; diff --git a/frontend/src/components/common/Loader.test.tsx b/frontend/src/components/common/Loader.test.tsx index ff8ceea2..f68642ba 100644 --- a/frontend/src/components/common/Loader.test.tsx +++ b/frontend/src/components/common/Loader.test.tsx @@ -1,39 +1,11 @@ import { render, textFragment } from 'flexible-testing-library-react'; -import mockElement from 'react-mock-element'; -import { css } from '../../test-helpers/queries'; -import { Loader } from './Loader'; +import { LoadingIndicator } from './Loader'; -const Component = mockElement('my-component'); - -describe('Loader', () => { - it('displays a loading message and no content while loading', () => { - const dom = render(); +describe('LoadingIndicator', () => { + it('displays a loading message', () => { + const dom = render(); expect(dom).toContainElementWith(textFragment('Loading')); - expect(dom).not.toContainElementWith(css('my-component')); - }); - - it('displays custom loading messages', () => { - const dom = render( - , - ); - - expect(dom).toContainElementWith(textFragment('foobar')); - }); - - it('displays content and no loading message when loaded', () => { - const dom = render( - , - ); - - expect(dom).not.toContainElementWith(textFragment('Loading')); - - const component = dom.getBy(css('my-component')); - expect(component.mockProps).toEqual({ custom: 'foo' }); }); }); diff --git a/frontend/src/components/common/Loader.tsx b/frontend/src/components/common/Loader.tsx index c98acb8b..bfcf4369 100644 --- a/frontend/src/components/common/Loader.tsx +++ b/frontend/src/components/common/Loader.tsx @@ -1,31 +1,10 @@ -import { - ReactNode, - ElementType, - ReactElement, - ComponentPropsWithRef, -} from 'react'; +import type { FC } from 'react'; import './Loader.less'; -interface PropsT { - Component: C; - componentProps: ComponentPropsWithRef | null; - loadingMessage?: ReactNode; - error?: string | null; -} +export const LoadingIndicator: FC = () => ( +
Loading…
+); -export const Loader = ({ - Component, - componentProps, - loadingMessage = 'Loading\u2026', - error, -}: PropsT): ReactElement => { - if (error) { - return
{error}
; - } - - if (!componentProps) { - return
{loadingMessage}
; - } - - return ; -}; +export const LoadingError: FC<{ error: string }> = ({ error }) => ( +
{error}
+); diff --git a/frontend/src/components/form.less b/frontend/src/components/form.less index 8509f251..708b9383 100644 --- a/frontend/src/components/form.less +++ b/frontend/src/components/form.less @@ -4,12 +4,34 @@ display: block; margin: 64px auto 128px; width: 600px; - max-width: calc(100% - 32px); + max-width: calc(100vw - 32px); label { display: block; } + .horizontal { + display: flex; + align-content: stretch; + + > label, + > input, + > .prefixed-input { + flex: 1 1 auto; + } + + input, + .prefixed-input { + margin: 0; + } + + > button { + flex: 0 0 auto; + width: auto; + margin: 0; + } + } + input, .prefixed-input, .picker-input { @@ -83,7 +105,7 @@ cursor: text; .prefix { - flex: initial; + flex: 0 1 auto; padding: 8px; padding-right: 2px; color: @pale-text-on-white; @@ -94,9 +116,7 @@ } input { - flex: 1; - width: 200px; - max-width: calc(100% - 10px); + flex: 1 0 200px; margin: 0; border: none; font-size: 1em; @@ -104,51 +124,6 @@ } } - .slug-checker { - position: absolute; - top: 100%; - right: 0; - margin: 4px; - font-size: 0.7rem; - white-space: nowrap; - - svg { - fill: currentColor; - width: 0.9em; - height: 0.9em; - vertical-align: -0.1em; - } - - &.checking { - margin-top: 4px; - border: 2px solid rgba(0, 0, 0, 0.5); - border-top-color: transparent; - border-bottom-color: transparent; - border-radius: 100%; - box-sizing: border-box; - width: 12px; - height: 12px; - animation: spin 2s linear infinite; - - @keyframes spin { - 100% { - transform: rotate(360deg); - } - } - } - - &.taken, - &.invalid { - color: @red-text-on-white; - font-weight: bolder; - } - - &.available { - color: @green-text-on-white; - font-weight: bolder; - } - } - .info { font-size: 0.8em; float: right; @@ -160,12 +135,12 @@ .sending { display: block; margin: 32px 0 0; - padding: 8px; + padding: 8px 16px; width: 100%; font-size: 1.2em; box-sizing: border-box; - background: @green-button-back; - color: #000000; + background: @green-button-dark; + color: #ffffff; text-align: center; &:disabled { @@ -173,10 +148,6 @@ } } - button[type='button'] { - background: white; - } - .sending { background: @beige-shadow; } diff --git a/frontend/src/components/hocs/withUserToken.tsx b/frontend/src/components/hocs/withUserToken.tsx deleted file mode 100644 index 9fe51b0d..00000000 --- a/frontend/src/components/hocs/withUserToken.tsx +++ /dev/null @@ -1,22 +0,0 @@ -import { ComponentType, ReactElement } from 'react'; -import { LoginForm } from '../login/LoginForm'; -import { useUserToken } from '../../hooks/data/useUserToken'; - -interface ChildPropsT { - userToken: string; -} - -type WrapperProps

= Omit; - -export const withUserToken = -

(message: string, Component: ComponentType

) => - (params: WrapperProps

): ReactElement => { - const [userToken] = useUserToken(); - - if (!userToken) { - return ; - } - - const AnyComponent = Component as ComponentType; - return ; - }; diff --git a/frontend/src/components/not-found/NotFoundPage.less b/frontend/src/components/not-found/NotFoundPage.less index fd23d97e..d9d14df3 100644 --- a/frontend/src/components/not-found/NotFoundPage.less +++ b/frontend/src/components/not-found/NotFoundPage.less @@ -1,2 +1,7 @@ .page-not-found { + text-align: center; + + p { + margin: 64px 32px; + } } diff --git a/frontend/src/components/password/PasswordPage.less b/frontend/src/components/password/PasswordPage.less index d3891359..0fa3d95e 100644 --- a/frontend/src/components/password/PasswordPage.less +++ b/frontend/src/components/password/PasswordPage.less @@ -25,8 +25,8 @@ width: 100%; font-size: 1.2em; box-sizing: border-box; - background: @green-button-back; - color: #000000; + background: @green-button-dark; + color: #ffffff; text-align: center; &:disabled { diff --git a/frontend/src/components/password/PasswordPage.test.tsx b/frontend/src/components/password/PasswordPage.test.tsx index 9299500d..42a79ac2 100644 --- a/frontend/src/components/password/PasswordPage.test.tsx +++ b/frontend/src/components/password/PasswordPage.test.tsx @@ -26,9 +26,8 @@ describe('PasswordPage', () => { const form = dom.getBy(css('form')); const fieldPassword = getBy(form, css('input[type=password]')); fireEvent.change(fieldPassword, { target: { value: 'my-password' } }); - await act(async () => { - fireEvent.submit(form); - }); + fireEvent.submit(form); + await act(() => Promise.resolve()); // fetch and store retro token const retroToken = await getToken('myRetroId'); expect(mockRetroTokenService.capturedPassword).toEqual('my-password'); diff --git a/frontend/src/components/retro-create/RetroCreatePage.less b/frontend/src/components/retro-create/RetroCreatePage.less deleted file mode 100644 index 084c94a6..00000000 --- a/frontend/src/components/retro-create/RetroCreatePage.less +++ /dev/null @@ -1,2 +0,0 @@ -.page-retro-create { -} diff --git a/frontend/src/components/retro-create/RetroCreatePage.tsx b/frontend/src/components/retro-create/RetroCreatePage.tsx index 29de04d3..88059f77 100644 --- a/frontend/src/components/retro-create/RetroCreatePage.tsx +++ b/frontend/src/components/retro-create/RetroCreatePage.tsx @@ -3,29 +3,50 @@ import { useLocation } from 'wouter'; import { Header } from '../common/Header'; import { useEvent } from '../../hooks/useEvent'; import { slugTracker } from '../../api/api'; +import { useUserToken } from '../../hooks/data/useUserToken'; +import { LoginForm } from '../login/LoginForm'; import { RetroForm, CreationT } from './RetroForm'; -import './RetroCreatePage.less'; interface PropsT { - defaultSlug?: string; + showImport?: boolean; } -export const RetroCreatePage = memo(({ defaultSlug }: PropsT) => { +export const RetroCreatePage = memo(({ showImport = false }: PropsT) => { + const [userToken] = useUserToken(); const [, setLocation] = useLocation(); const handleCreate = useEvent(({ id, slug }: CreationT) => { slugTracker.set(slug, id); setLocation(`/retros/${encodeURIComponent(slug)}`); }); + const title = showImport ? 'Import Retro' : 'New Retro'; + + if (!userToken) { + return ( +

+
+ +
+ ); + } + return (
+ -
); }); diff --git a/frontend/src/components/retro-create/RetroForm.tsx b/frontend/src/components/retro-create/RetroForm.tsx index b6ca31ff..6cce6169 100644 --- a/frontend/src/components/retro-create/RetroForm.tsx +++ b/frontend/src/components/retro-create/RetroForm.tsx @@ -2,9 +2,9 @@ import { useState, memo, ChangeEvent, ReactNode } from 'react'; import { type JsonData } from '../../shared/api-entities'; import { useEvent } from '../../hooks/useEvent'; import { Input } from '../common/Input'; -import { SlugEntry, MAX_SLUG_LENGTH } from './SlugEntry'; +import { SlugEntry } from './SlugEntry'; import { Alert } from '../common/Alert'; -import { withUserToken } from '../hocs/withUserToken'; +import { makeValidSlug } from '../../hooks/data/useSlugAvailability'; import { useSubmissionCallback } from '../../hooks/useSubmissionCallback'; import { useNonce } from '../../hooks/useNonce'; import { @@ -31,16 +31,6 @@ interface PropsT { const MIN_PASSWORD_LENGTH = 8; const MAX_PASSWORD_LENGTH = 512; -function makeSlug(text: string): string { - return text - .toLowerCase() - .replace(/['"]/g, '') - .replace(/[^a-z0-9_]+/g, '-') - .replace(/[-_]{2,}/g, '_') - .replace(/^[-_]+|[-_]+$/g, '') - .substring(0, MAX_SLUG_LENGTH); -} - function readFileText(file: File): Promise { return new Promise((resolve, reject) => { const reader = new FileReader(); @@ -55,204 +45,203 @@ function readFileText(file: File): Promise { } export const RetroForm = memo( - withUserToken( - 'Register an account to create a retro', - ({ defaultSlug, userToken, onCreate, showImport = false }: PropsT) => { - const [name, setName] = useState(defaultSlug || ''); - const [slug, setSlug] = useState(defaultSlug || ''); - const [password, setPassword] = useState(''); - const [passwordConf, setPasswordConfRaw] = useState(''); - const [passwordWarning, setPasswordWarning] = useState(); - const [importJson, setImportJson] = useState(); - const [importError, setImportError] = useState(); - - const importNonce = useNonce(); - const handleImportChange = useEvent( - async (e: ChangeEvent) => { - const file = (e.target.files || [])[0]; - setImportJson(undefined); - setImportError(undefined); - if (!file) { + ({ defaultSlug, userToken, onCreate, showImport = false }: PropsT) => { + const [name, setName] = useState(defaultSlug || ''); + const [slug, setSlug] = useState(defaultSlug || ''); + const [password, setPassword] = useState(''); + const [passwordConf, setPasswordConfRaw] = useState(''); + const [passwordWarning, setPasswordWarning] = useState(); + const [importJson, setImportJson] = useState(); + const [importError, setImportError] = useState(); + + const importNonce = useNonce(); + const handleImportChange = useEvent( + async (e: ChangeEvent) => { + const file = (e.target.files || [])[0]; + setImportJson(undefined); + setImportError(undefined); + if (!file) { + return; + } + const nonce = importNonce.next(); + try { + const content = await readFileText(file); + if (!importNonce.check(nonce)) { return; } - const nonce = importNonce.next(); - try { - const content = await readFileText(file); - if (!importNonce.check(nonce)) { - return; - } - const json = JSON.parse(content); - setName(String(json.name || '')); - setSlug(String(json.url || '')); - setImportJson(json); - } catch (err: unknown) { - if (err instanceof Error) { - setImportError(err.message); - } else { - setImportError('Internal error'); - } + const json = JSON.parse(content); + setName(String(json.name || '')); + setSlug(String(json.url || '')); + setImportJson(json); + } catch (err: unknown) { + if (err instanceof Error) { + setImportError(err.message); + } else { + setImportError('Internal error'); } - }, - ); + } + }, + ); - const passwordCheckNonce = useNonce(); - const setPasswordConf = useEvent(async (current: string) => { - const nonce = passwordCheckNonce.next(); - setPasswordConfRaw(current); - setPasswordWarning(undefined); + const passwordCheckNonce = useNonce(); + const setPasswordConf = useEvent(async (current: string) => { + const nonce = passwordCheckNonce.next(); + setPasswordConfRaw(current); + setPasswordWarning(undefined); - if (password !== current) { - return; - } + if (password !== current) { + return; + } - if (current.length < MIN_PASSWORD_LENGTH) { - setPasswordWarning('This password is too short'); - return; - } - if (current.length > MAX_PASSWORD_LENGTH) { - setPasswordWarning('This password is too long'); - return; - } + if (current.length < MIN_PASSWORD_LENGTH) { + setPasswordWarning('This password is too short'); + return; + } + if (current.length > MAX_PASSWORD_LENGTH) { + setPasswordWarning('This password is too long'); + return; + } - try { - const count = await passwordService.countPasswordBreaches(current); - if (!passwordCheckNonce.check(nonce)) { - return; - } - if (count > 100) { - setPasswordWarning('This password is very common and insecure'); - } else if (count > 20) { - setPasswordWarning('This password is common and insecure'); - } else if (count > 0) { - setPasswordWarning('This password may be insecure'); - } - } catch (err) { - // ignore + try { + const count = await passwordService.countPasswordBreaches(current); + if (!passwordCheckNonce.check(nonce)) { + return; } - }); - - const [handleSubmit, sending, error] = useSubmissionCallback(async () => { - if (showImport && !importJson) { - throw new Error('Must specify valid file to import'); + if (count > 100) { + setPasswordWarning('This password is very common and insecure'); + } else if (count > 20) { + setPasswordWarning('This password is common and insecure'); + } else if (count > 0) { + setPasswordWarning('This password may be insecure'); } + } catch (err) { + // ignore + } + }); - const resolvedSlug = slug || makeSlug(name); - if (!name || !password || !resolvedSlug) { - throw new Error('Must specify name, password and URL'); - } + const [handleSubmit, sending, error] = useSubmissionCallback(async () => { + if (showImport && !importJson) { + throw new Error('Must specify valid file to import'); + } - if (password !== passwordConf) { - throw new Error('Passwords do not match'); - } + const resolvedSlug = slug || makeValidSlug(name); + if (!name || !password || !resolvedSlug) { + throw new Error('Must specify name, password and URL'); + } - const { id, token } = await retroService.create({ - name, - slug: resolvedSlug, - password, - userToken, - importJson, - }); + if (password !== passwordConf) { + throw new Error('Passwords do not match'); + } - retroTokenTracker.set(id, token); - onCreate({ - id, - slug: resolvedSlug, - name, - password, - }); + const { id, token } = await retroService.create({ + name, + slug: resolvedSlug, + password, + userToken, + importJson, }); - let importComponent: ReactNode = null; - if (showImport) { - importComponent = ( - - ); - } + retroTokenTracker.set(id, token); + onCreate({ + id, + slug: resolvedSlug, + name, + password, + }); + }); - return ( -
- {importComponent} - - - - - - - {' \u2014 '} - - Learn more - - - {sending ? ( -
…
- ) : ( - - )} - - + let importComponent: ReactNode = null; + if (showImport) { + importComponent = ( + ); - }, - ), + } + + return ( +
+ {importComponent} + + + + + + + {' \u2014 '} + + Learn more + + + {sending ? ( +
…
+ ) : ( + + )} + + + ); + }, ); diff --git a/frontend/src/components/retro-create/RetroImportPage.less b/frontend/src/components/retro-create/RetroImportPage.less deleted file mode 100644 index 5a878682..00000000 --- a/frontend/src/components/retro-create/RetroImportPage.less +++ /dev/null @@ -1,2 +0,0 @@ -.page-retro-import { -} diff --git a/frontend/src/components/retro-create/RetroImportPage.tsx b/frontend/src/components/retro-create/RetroImportPage.tsx deleted file mode 100644 index c1e157b6..00000000 --- a/frontend/src/components/retro-create/RetroImportPage.tsx +++ /dev/null @@ -1,27 +0,0 @@ -import { memo } from 'react'; -import { useLocation } from 'wouter'; -import { Header } from '../common/Header'; -import { useEvent } from '../../hooks/useEvent'; -import { slugTracker } from '../../api/api'; -import { RetroForm, CreationT } from './RetroForm'; -import './RetroImportPage.less'; - -export const RetroImportPage = memo(() => { - const [, setLocation] = useLocation(); - const handleCreate = useEvent(({ id, slug }: CreationT) => { - slugTracker.set(slug, id); - setLocation(`/retros/${encodeURIComponent(slug)}`); - }); - - return ( -
-
- -
- ); -}); diff --git a/frontend/src/components/retro-create/SlugEntry.less b/frontend/src/components/retro-create/SlugEntry.less new file mode 100644 index 00000000..7587f818 --- /dev/null +++ b/frontend/src/components/retro-create/SlugEntry.less @@ -0,0 +1,46 @@ +@import (reference) '../colours.less'; + +.slug-checker { + position: absolute; + top: 100%; + right: 0; + margin: 4px; + font-size: 0.7rem; + white-space: nowrap; + + svg { + fill: currentColor; + width: 0.9em; + height: 0.9em; + vertical-align: -0.1em; + } + + &.checking { + margin-top: 4px; + border: 2px solid rgba(0, 0, 0, 0.5); + border-top-color: transparent; + border-bottom-color: transparent; + border-radius: 100%; + box-sizing: border-box; + width: 12px; + height: 12px; + animation: spin 2s linear infinite; + + @keyframes spin { + 100% { + transform: rotate(360deg); + } + } + } + + &.taken, + &.invalid { + color: @red-text-on-white; + font-weight: bolder; + } + + &.available { + color: @green-text-on-white; + font-weight: bolder; + } +} diff --git a/frontend/src/components/retro-create/SlugEntry.tsx b/frontend/src/components/retro-create/SlugEntry.tsx index 83e6cf24..ad66828a 100644 --- a/frontend/src/components/retro-create/SlugEntry.tsx +++ b/frontend/src/components/retro-create/SlugEntry.tsx @@ -1,30 +1,30 @@ -import { memo, ReactNode } from 'react'; -import useAwaited from 'react-hook-awaited'; +import { ReactNode, type FC } from 'react'; import { Input } from '../common/Input'; -import { slugTracker } from '../../api/api'; import TickBold from '../../../resources/tick-bold.svg'; import Cross from '../../../resources/cross.svg'; - -export const MAX_SLUG_LENGTH = 64; -const VALID_SLUG_PATTERN = '^[a-z0-9][a-z0-9_\\-]*$'; -const VALID_SLUG = new RegExp(VALID_SLUG_PATTERN); - -enum SlugAvailability { - BLANK, - INVALID, - TAKEN, - AVAILABLE, -} +import { + MAX_SLUG_LENGTH, + SlugAvailability, + useSlugAvailability, + VALID_SLUG_PATTERN, +} from '../../hooks/data/useSlugAvailability'; +import './SlugEntry.less'; interface PropsT { placeholder?: string; + ariaLabel?: string; value: string; onChange: (v: string) => void; oldValue?: string; + showAvailability?: boolean; } const availabilityJsx: Record = { [SlugAvailability.BLANK]: null, + [SlugAvailability.PENDING]:
, + [SlugAvailability.FAILED]: ( +
Unable to check availability
+ ), [SlugAvailability.INVALID]: (
Invalid @@ -42,60 +42,40 @@ const availabilityJsx: Record = { ), }; -export const SlugEntry = memo( - ({ placeholder = '', value, onChange, oldValue }: PropsT) => { - const active = value || placeholder; - const slugAvailability = useAwaited(async () => { - if (active === '') { - return SlugAvailability.BLANK; - } - if (active === oldValue) { - return SlugAvailability.AVAILABLE; - } - if (!VALID_SLUG.test(active) || active.length > MAX_SLUG_LENGTH) { - return SlugAvailability.INVALID; - } - if (!(await slugTracker.isAvailable(active))) { - return SlugAvailability.TAKEN; - } - return SlugAvailability.AVAILABLE; - }, [active, slugTracker, oldValue]); - - const retrosBaseUrl = `${document.location.host}/retros/`; - - let slugChecker: ReactNode; - switch (slugAvailability.state) { - case 'pending': - slugChecker =
; - break; - case 'rejected': - slugChecker = ( -
- Unable to check availability -
- ); - break; - case 'resolved': - slugChecker = availabilityJsx[slugAvailability.data]; - break; - // no default - } +const SlugAvailabilityDisplay = ({ + slug, + oldValue, +}: { + slug: string; + oldValue: string | undefined; +}) => availabilityJsx[useSlugAvailability(slug, oldValue)]; - return ( -
- {retrosBaseUrl} - - {slugChecker} -
- ); - }, +export const SlugEntry: FC = ({ + placeholder = '', + ariaLabel, + value, + onChange, + oldValue, + showAvailability = false, +}) => ( +
+ {`${document.location.host}/retros/`} + + {showAvailability ? ( + + ) : null} +
); diff --git a/frontend/src/components/retro-list/RetroList.tsx b/frontend/src/components/retro-list/RetroList.tsx deleted file mode 100644 index bcfbd0cd..00000000 --- a/frontend/src/components/retro-list/RetroList.tsx +++ /dev/null @@ -1,23 +0,0 @@ -import { memo } from 'react'; -import { type RetroSummary } from '../../shared/api-entities'; -import { RetroLink } from './RetroLink'; - -interface PropsT { - retros: RetroSummary[]; -} - -export const RetroList = memo(({ retros }: PropsT) => { - if (!retros.length) { - return

You do not have any retros yet!

; - } - - return ( -
    - {retros.map(({ id, slug, name }) => ( -
  • - -
    • - ))} -
- ); -}); diff --git a/frontend/src/components/retro-list/RetroListPage.less b/frontend/src/components/retro-list/RetroListPage.less deleted file mode 100644 index 17c17dba..00000000 --- a/frontend/src/components/retro-list/RetroListPage.less +++ /dev/null @@ -1,27 +0,0 @@ -@import (reference) '../colours.less'; - -.page-retro-list { - ul.retros { - margin: 64px 16px 128px; - list-style: none; - text-align: center; - - li a { - display: inline-block; - margin: 8px 0; - padding: 16px; - width: 20rem; - max-width: calc(100% - 32px); - background: #ffffff; - color: @green-text-on-white; - - &:active, - &:hover, - &:focus { - color: #ffffff; - text-decoration: none; - background: @green-button-dark; - } - } - } -} diff --git a/frontend/src/components/retro-list/RetroListPage.test.tsx b/frontend/src/components/retro-list/RetroListPage.test.tsx deleted file mode 100644 index 882d4956..00000000 --- a/frontend/src/components/retro-list/RetroListPage.test.tsx +++ /dev/null @@ -1,29 +0,0 @@ -import { render } from 'flexible-testing-library-react'; -import mockElement from 'react-mock-element'; -import { retroListTracker } from '../../api/api'; -import type * as mockApiTypes from '../../api/__mocks__/api'; -import { css } from '../../test-helpers/queries'; - -import { RetroListPage } from './RetroListPage'; - -jest.mock('../../api/api'); -jest.mock('../common/Header', () => ({ Header: mockElement('mock-header') })); -jest.mock('./RetroList', () => ({ RetroList: mockElement('mock-retro-list') })); - -const mockRetroListTracker = - retroListTracker as unknown as typeof mockApiTypes.retroListTracker; - -describe('RetroListPage', () => { - beforeEach(() => { - mockRetroListTracker.set('foobar', { - retros: [{ id: 'u1', slug: 'a', name: 'R1' }], - }); - }); - - it('loads data when displayed', () => { - const dom = render(); - - const retroList = dom.getBy(css('mock-retro-list')); - expect(retroList.mockProps['retros'].length).toEqual(1); - }); -}); diff --git a/frontend/src/components/retro-list/RetroListPage.tsx b/frontend/src/components/retro-list/RetroListPage.tsx deleted file mode 100644 index aae2d29c..00000000 --- a/frontend/src/components/retro-list/RetroListPage.tsx +++ /dev/null @@ -1,28 +0,0 @@ -import { memo } from 'react'; -import { Header } from '../common/Header'; -import { Loader } from '../common/Loader'; -import { useRetroList } from '../../hooks/data/useRetroList'; -import { RetroList } from './RetroList'; -import './RetroListPage.less'; - -export const RetroListPage = memo(({ userToken }: { userToken: string }) => { - const [retroList, error] = useRetroList(userToken); - - return ( -
-
- -
- ); -}); diff --git a/frontend/src/components/retro-not-found/RetroNotFoundPage.tsx b/frontend/src/components/retro-not-found/RetroNotFoundPage.tsx new file mode 100644 index 00000000..70d3ca81 --- /dev/null +++ b/frontend/src/components/retro-not-found/RetroNotFoundPage.tsx @@ -0,0 +1,45 @@ +import { memo } from 'react'; +import { useLocation } from 'wouter'; +import { slugTracker } from '../../api/api'; +import { useEvent } from '../../hooks/useEvent'; +import { useUserToken } from '../../hooks/data/useUserToken'; +import { LoginForm } from '../login/LoginForm'; +import { RetroForm, type CreationT } from '../retro-create/RetroForm'; +import { Header } from '../common/Header'; + +interface PropsT { + slug: string; +} + +export const RetroNotFoundPage = memo(({ slug }: PropsT) => { + const [userToken] = useUserToken(); + const [, setLocation] = useLocation(); + const handleCreate = useEvent(({ id, slug }: CreationT) => { + slugTracker.set(slug, id); + setLocation(`/retros/${encodeURIComponent(slug)}`); + }); + + if (!userToken) { + return ( +
+
+ +
+ ); + } + + return ( +
+
+ +
+ ); +}); diff --git a/frontend/src/components/retro-settings/RetroSettingsPage.test.tsx b/frontend/src/components/retro-settings/RetroSettingsPage.test.tsx index 9b41765b..983c16d7 100644 --- a/frontend/src/components/retro-settings/RetroSettingsPage.test.tsx +++ b/frontend/src/components/retro-settings/RetroSettingsPage.test.tsx @@ -11,14 +11,12 @@ jest.mock('../common/Header', () => ({ Header: mockElement('mock-header') })); describe('RetroSettingsPage', () => { it('renders basic settings', async () => { - let dom; - await act(async () => { - dom = render( - - - , - ); - }); + const dom = render( + + + , + ); + await act(() => Promise.resolve()); // slug availability update expect(dom).toContainElementWith(placeholderText('retro name')); }); diff --git a/frontend/src/components/retro-settings/SettingsForm.tsx b/frontend/src/components/retro-settings/SettingsForm.tsx index 20d609db..54a1da2c 100644 --- a/frontend/src/components/retro-settings/SettingsForm.tsx +++ b/frontend/src/components/retro-settings/SettingsForm.tsx @@ -73,7 +73,13 @@ export const SettingsForm = memo(({ retro, dispatch, onSave }: PropsT) => {
(may contain lowercase letters, numbers, dashes and underscores)
- +