diff --git a/e2e/account.spec.ts b/e2e/account.spec.ts index 84990701..7d034f2f 100644 --- a/e2e/account.spec.ts +++ b/e2e/account.spec.ts @@ -39,9 +39,43 @@ test.describe('Account', () => { await login.getByRole('button', { name: 'Login' }).click(); await expect(page).toHaveURL(/\/login$/); + await expect(login.getByText('Password is required')).toBeVisible(); + await expect(login.getByText('Username is required')).toHaveCount(0); await expectLoggedOut(page); }); + for (const form of [ + { menu: 'Login', pageId: 'login-page', button: 'Login' }, + { menu: 'Signup', pageId: 'signup-page', button: 'Create' }, + ]) { + test(`${form.menu} shows the required messages when submitted empty`, async ({ page }) => { + await navigateFromMenu(page, form.menu); + const formPage = visiblePage(page, form.pageId); + await expect(formPage.getByText('Username is required')).toBeHidden(); + + await formPage.getByRole('button', { name: form.button }).click(); + + await expect(formPage.getByText('Username is required')).toBeVisible(); + await expect(formPage.getByText('Password is required')).toBeVisible(); + await expect(page).toHaveURL(new RegExp(`/${form.menu.toLowerCase()}$`)); + }); + + test(`${form.menu} shows a required message when an empty field is left`, async ({ page }) => { + await navigateFromMenu(page, form.menu); + const formPage = visiblePage(page, form.pageId); + const username = formPage.getByRole('textbox', { name: 'Username' }); + + await username.focus(); + await username.blur(); + + await expect(formPage.getByText('Username is required')).toBeVisible(); + await expect(formPage.getByText('Password is required')).toBeHidden(); + + await username.fill('ionitron'); + await expect(formPage.getByText('Username is required')).toBeHidden(); + }); + } + test('logging in marks the user as logged in', async ({ page }) => { await login(page, 'ionitron'); diff --git a/e2e/helpers.ts b/e2e/helpers.ts index 71c6040f..968cb0ae 100644 --- a/e2e/helpers.ts +++ b/e2e/helpers.ts @@ -25,15 +25,16 @@ export const visibleTitle = (page: Page): Locator => page.locator('ion-title').f export const sessionLink = (page: Page, name: string): Locator => visiblePage(page, 'schedule-page').getByRole('link', { name: new RegExp(`^${escapeRegExp(name)} \\d`) }); -// The schedule's toolbar buttons only show an icon and have no accessible name, so find them by position. -const scheduleEndButtons = (page: Page): Locator => - visiblePage(page, 'schedule-page').locator('ion-header ion-buttons[slot="end"] ion-button'); -export const searchButton = (page: Page): Locator => scheduleEndButtons(page).first(); -export const filterButton = (page: Page): Locator => scheduleEndButtons(page).last(); +export const searchButton = (page: Page): Locator => + visiblePage(page, 'schedule-page').getByRole('button', { name: 'Search', exact: true }); +export const filterButton = (page: Page): Locator => + visiblePage(page, 'schedule-page').getByRole('button', { name: 'Filter', exact: true }); -// The session detail's favorite button is the first icon-only button at the end of its toolbar. +// The session detail's favorite button is labelled with what pressing it does. export const favoriteButton = (page: Page): Locator => - visiblePage(page, 'session-detail-page').locator('ion-buttons[slot="end"] ion-button').first(); + visiblePage(page, 'session-detail-page').getByRole('button', { name: 'Favorite', exact: true }); +export const removeFavoriteButton = (page: Page): Locator => + visiblePage(page, 'session-detail-page').getByRole('button', { name: 'Remove favorite', exact: true }); export const openMenu = async (page: Page): Promise => { // Wait for any page transition to finish so only the current page's menu button is showing. diff --git a/e2e/navigation.spec.ts b/e2e/navigation.spec.ts index 3bf9a762..312f2874 100644 --- a/e2e/navigation.spec.ts +++ b/e2e/navigation.spec.ts @@ -72,7 +72,6 @@ test.describe('Navigation', () => { const support = visiblePage(page, 'support-page'); const notice = page.getByText('This does not actually send a support request.'); await expect(notice).toBeVisible(); - // The page can only show one toast at a time and skips the confirmation while the notice is open (see the report). await expect(notice).toBeHidden({ timeout: 10_000 }); const message = support.getByRole('textbox', { name: 'Enter your support message below' }); @@ -83,15 +82,61 @@ test.describe('Navigation', () => { await expect(message).toHaveValue(''); }); + test('the support page shows the confirmation even while the notice is still open', async ({ page }) => { + await navigateFromMenu(page, 'Support'); + const support = visiblePage(page, 'support-page'); + const notice = page.getByText('This does not actually send a support request.'); + await expect(notice).toBeVisible(); + + await support.getByRole('textbox', { name: 'Enter your support message below' }).fill('The app is great!'); + await support.getByRole('button', { name: 'Submit' }).click(); + + await expect(page.getByText('Your support request has been sent.')).toBeVisible(); + await expect(notice).toBeHidden(); + }); + test('the support page does not send an empty message', async ({ page }) => { await navigateFromMenu(page, 'Support'); const support = visiblePage(page, 'support-page'); + await expect(support.getByText('Support message is required')).toBeHidden(); await support.getByRole('button', { name: 'Submit' }).click(); - // The "Support message is required" error text isn't shown (see the report), so check nothing was sent. - await expect(support.getByRole('textbox', { name: 'Enter your support message below' })).toBeVisible(); + await expect(support.getByText('Support message is required')).toBeVisible(); await expect(page.getByText('Your support request has been sent.')).toHaveCount(0); + + // Typing a message clears the error. + await support.getByRole('textbox', { name: 'Enter your support message below' }).fill('Hello'); + await expect(support.getByText('Support message is required')).toBeHidden(); + }); + + test('the support message shows its required message when left empty', async ({ page }) => { + await navigateFromMenu(page, 'Support'); + const support = visiblePage(page, 'support-page'); + const message = support.getByRole('textbox', { name: 'Enter your support message below' }); + + await message.focus(); + await message.blur(); + + await expect(support.getByText('Support message is required')).toBeVisible(); + }); + + test('the share button opens the social share buttons', async ({ page }) => { + const schedule = visiblePage(page, 'schedule-page'); + await schedule.getByRole('button', { name: 'Share', exact: true }).click(); + for (const name of ['Instagram', 'Twitter', 'Facebook']) { + await expect(schedule.getByRole('button', { name, exact: true })).toBeVisible(); + } + await schedule.getByRole('button', { name: 'Vimeo', exact: true }).click(); + + await expect(page.getByText('Posting to Vimeo')).toBeVisible(); + }); + + test('the About page opens its options from the More options button', async ({ page }) => { + await page.getByRole('tab', { name: 'About' }).click(); + await visiblePage(page, 'about-page').getByRole('button', { name: 'More options', exact: true }).click(); + + await expect(page.getByText('Learn Ionic')).toBeVisible(); }); test('the dark mode toggle switches the dark palette', async ({ page }) => { diff --git a/e2e/session-detail.spec.ts b/e2e/session-detail.spec.ts index 7010e420..c0e417dd 100644 --- a/e2e/session-detail.spec.ts +++ b/e2e/session-detail.spec.ts @@ -1,16 +1,24 @@ import type { Page } from '@playwright/test'; import { expect, test } from '@playwright/test'; -import { backButton, favoriteButton, openSchedule, showSegment, sessionLink, visiblePage } from './helpers'; +import { + backButton, + favoriteButton, + openSchedule, + removeFavoriteButton, + showSegment, + sessionLink, + visiblePage, +} from './helpers'; const detail = (page: Page) => visiblePage(page, 'session-detail-page'); const schedule = (page: Page) => visiblePage(page, 'schedule-page'); -const toggleFavoriteFromDetail = async (page: Page, name: string) => { +const toggleFavoriteFromDetail = async (page: Page, name: string, button: typeof favoriteButton) => { await showSegment(page, 'All'); await sessionLink(page, name).click(); await expect(detail(page).getByRole('heading', { level: 1 })).toHaveText(name); - await favoriteButton(page).click(); + await button(page).click(); await backButton(page, 'session-detail-page').click(); await showSegment(page, 'Favorites'); }; @@ -19,6 +27,8 @@ const toggleFavoriteFromDetail = async (page: Page, name: string) => { const swipeOpen = async (page: Page, name: string) => { const item = sessionLink(page, name); await expect(item).toBeVisible(); + // Wait for an item that was just closed to finish closing, or the swipe is lost. + await expect(schedule(page).locator('ion-item-sliding.item-sliding-active-slide')).toHaveCount(0); const box = (await item.boundingBox())!; const y = box.y + box.height / 2; await page.mouse.move(box.x + box.width - 20, y); @@ -27,6 +37,8 @@ const swipeOpen = async (page: Page, name: string) => { await page.mouse.up(); }; +const visibleSessions = (page: Page) => schedule(page).getByRole('link').filter({ visible: true }); + const itemOption = (page: Page, name: string) => schedule(page).getByRole('button', { name, exact: true }).filter({ visible: true }); @@ -47,16 +59,27 @@ test.describe('Session detail', () => { }); test('favoriting from the detail page adds the session to Favorites', async ({ page }) => { - await toggleFavoriteFromDetail(page, 'Ionic Tooling'); + await toggleFavoriteFromDetail(page, 'Ionic Tooling', favoriteButton); await expect(sessionLink(page, 'Ionic Tooling')).toBeVisible(); }); + test("the favorite button's label says whether it adds or removes the favorite", async ({ page }) => { + await sessionLink(page, 'Ionic Tooling').click(); + await expect(removeFavoriteButton(page)).toHaveCount(0); + + await favoriteButton(page).click(); + + await expect(removeFavoriteButton(page)).toBeVisible(); + await expect(favoriteButton(page)).toHaveCount(0); + await expect(detail(page).getByRole('button', { name: 'Share', exact: true })).toBeVisible(); + }); + test('unfavoriting from the detail page removes the session from Favorites', async ({ page }) => { - await toggleFavoriteFromDetail(page, 'Ionic Tooling'); + await toggleFavoriteFromDetail(page, 'Ionic Tooling', favoriteButton); await expect(sessionLink(page, 'Ionic Tooling')).toBeVisible(); - await toggleFavoriteFromDetail(page, 'Ionic Tooling'); + await toggleFavoriteFromDetail(page, 'Ionic Tooling', removeFavoriteButton); await expect(sessionLink(page, 'Ionic Tooling')).toBeHidden(); await expect(schedule(page).getByText('No Sessions Found')).toBeVisible(); @@ -86,4 +109,41 @@ test.describe('Session detail', () => { await expect(sessionLink(page, 'Ionic Tooling')).toBeHidden(); await expect(schedule(page).getByText('No Sessions Found')).toBeVisible(); }); + + test('favoriting a session that is already a favorite asks to remove it, and Cancel keeps it', async ({ page }) => { + await swipeOpen(page, 'Ionic Tooling'); + await itemOption(page, 'Favorite').click(); + await expect(page.getByText('Ionic Tooling was successfully added as a favorite.')).toBeVisible(); + + await swipeOpen(page, 'Ionic Tooling'); + await itemOption(page, 'Favorite').click(); + const alert = page.getByRole('alertdialog', { name: 'Favorite already added' }); + await alert.getByRole('button', { name: 'Cancel' }).click(); + await expect(alert).toBeHidden(); + + await showSegment(page, 'Favorites'); + await expect(visibleSessions(page)).toHaveCount(1); + await expect(sessionLink(page, 'Ionic Tooling')).toBeVisible(); + + // Removing it once removes it, so it wasn't added a second time. + await swipeOpen(page, 'Ionic Tooling'); + await itemOption(page, 'Remove').click(); + await page.getByRole('alertdialog', { name: 'Remove Favorite' }).getByRole('button', { name: 'Remove' }).click(); + await expect(schedule(page).getByText('No Sessions Found')).toBeVisible(); + }); + + test('Remove in the already-a-favorite prompt unfavorites the session', async ({ page }) => { + await swipeOpen(page, 'Ionic Tooling'); + await itemOption(page, 'Favorite').click(); + + await swipeOpen(page, 'Ionic Tooling'); + await itemOption(page, 'Favorite').click(); + await page + .getByRole('alertdialog', { name: 'Favorite already added' }) + .getByRole('button', { name: 'Remove' }) + .click(); + + await showSegment(page, 'Favorites'); + await expect(schedule(page).getByText('No Sessions Found')).toBeVisible(); + }); }); diff --git a/e2e/speakers.spec.ts b/e2e/speakers.spec.ts index 3c166eb9..770128f0 100644 --- a/e2e/speakers.spec.ts +++ b/e2e/speakers.spec.ts @@ -32,6 +32,8 @@ test.describe('Speakers', () => { const detail = visiblePage(page, 'speaker-detail'); await expect(detail.getByRole('heading', { name: 'Burt Bear' })).toBeVisible(); await expect(detail).toContainText('Burt is a Bear.'); + await expect(detail.getByRole('button', { name: 'Contact', exact: true })).toBeVisible(); + await expect(detail.getByRole('button', { name: 'Share', exact: true })).toBeVisible(); }); test("opens a speaker's session from the speaker list", async ({ page }) => { diff --git a/renovate.json b/renovate.json index 93bad409..816a408e 100644 --- a/renovate.json +++ b/renovate.json @@ -11,6 +11,11 @@ "semanticCommits": "enabled", "ignorePaths": ["android/**", "ios/**"], "gitIgnoredAuthors": ["hi@ionicframework.com"], + "platformAutomerge": false, + "lockFileMaintenance": { + "enabled": true, + "automerge": true + }, "packageRules": [ { "matchPackageNames": ["@ionic/**", "!@ionic/eslint-config", "!@ionic/prettier-config"], @@ -54,6 +59,18 @@ "typescript" ], "groupName": "dev-tools" + }, + { + "description": "Automerge minor and patch updates once CI passes. Majors and 0.x packages, where a minor can break, still need review.", + "matchUpdateTypes": ["minor", "patch"], + "matchCurrentVersion": "!/^0/", + "automerge": true + }, + { + "description": "CI only tests the web build, so Capacitor updates still need review for native changes. Pinned so lockfile maintenance can't update it either.", + "matchPackageNames": ["@capacitor/**"], + "rangeStrategy": "pin", + "automerge": false } ] } diff --git a/src/components/SessionList.test.tsx b/src/components/SessionList.test.tsx new file mode 100644 index 00000000..84167f88 --- /dev/null +++ b/src/components/SessionList.test.tsx @@ -0,0 +1,51 @@ +import { fireEvent, screen } from '@testing-library/react'; +import { useContext } from 'react'; +import { describe, expect, it } from 'vitest'; + +import { AppContext } from '../data/AppContext'; +import { getFilteredSchedule } from '../data/selectors'; +import { breakfast, ionicTooling, makeState } from '../testing/fixtures'; +import { renderWithState } from '../testing/render'; + +import SessionList from './SessionList'; + +// Shows the favorited session ids from the app state. +const Favorites = () => {useContext(AppContext).state.data.favorites.join(',')}; + +const renderList = (favorites: number[]) => { + const state = makeState({ data: { favorites } }); + return renderWithState( + <> + + + , + { state }, + ); +}; + +// The Favorite option revealed by swiping a session's item. +const favoriteOption = (container: HTMLElement, name: string) => { + const item = Array.from(container.querySelectorAll('ion-item-sliding')).find( + (sliding) => sliding.querySelector('h3')?.textContent === name, + )!; + return item.querySelector('ion-item-option') as HTMLElement; +}; + +describe('SessionList', () => { + it('favorites a session from its item', () => { + const { container } = renderList([]); + + fireEvent.click(favoriteOption(container, 'Ionic Tooling')); + + expect(screen.getByRole('status')).toHaveTextContent(String(ionicTooling.id)); + }); + + it('does not favorite a session twice when it is already a favorite', () => { + const { container } = renderList([ionicTooling.id, breakfast.id]); + + // This asks whether to remove the favorite instead, and leaves the favorites alone until it is answered. + fireEvent.click(favoriteOption(container, 'Ionic Tooling')); + + expect(screen.getByRole('status')).toHaveTextContent(`${ionicTooling.id},${breakfast.id}`); + }); +}); diff --git a/src/components/SessionListItem.tsx b/src/components/SessionListItem.tsx index f1af911b..f38c86d5 100644 --- a/src/components/SessionListItem.tsx +++ b/src/components/SessionListItem.tsx @@ -29,7 +29,6 @@ const SessionListItem: React.FC = ({ }; const removeFavoriteSession = (title: string) => { - onAddFavorite(session.id); onShowAlert(title, 'Would you like to remove this session from your favorites?', [ { text: 'Cancel', diff --git a/src/components/ShareSocialFab.tsx b/src/components/ShareSocialFab.tsx index 210c66eb..77d4e4ec 100644 --- a/src/components/ShareSocialFab.tsx +++ b/src/components/ShareSocialFab.tsx @@ -21,20 +21,20 @@ const ShareSocialFab: React.FC = () => { onDidDismiss={() => setShowLoading(false)} /> - + - openSocial('Vimeo')}> + openSocial('Vimeo')}> - openSocial('Instagram')}> + openSocial('Instagram')}> - openSocial('Twitter')}> + openSocial('Twitter')}> - openSocial('Facebook')}> + openSocial('Facebook')}> diff --git a/src/data/selectors.test.ts b/src/data/selectors.test.ts index c4441069..fd2c3a4a 100644 --- a/src/data/selectors.test.ts +++ b/src/data/selectors.test.ts @@ -58,6 +58,39 @@ describe('getFilteredSchedule', () => { expect(getFilteredSchedule(state).groups.map((group) => group.time)).toEqual(['12:00 am', '11:00 am', '12:30 pm']); }); + it('sorts by the minutes as numbers, so 9:05 comes before 9:30 and 10:00', () => { + const at = (id: number, timeStart: string) => makeSession({ id, name: timeStart, timeStart }); + const state = makeState({ + data: { + schedule: { + date: '2047-05-17', + groups: [ + { time: '10:00 am', sessions: [at(10, '10:00 am')] }, + { time: '9:05 am', sessions: [at(11, '9:30 am'), at(12, '9:05 am')] }, + ], + }, + }, + }); + + expect(summarize(getFilteredSchedule(state))).toEqual([ + ['9:05 am', ['9:05 am', '9:30 am']], + ['10:00 am', ['10:00 am']], + ]); + }); + + it('lists a session once when more than one of its tracks is selected', () => { + const workshop = makeSession({ id: 10, name: 'Ionic React Workshop', tracks: ['Ionic', 'React'] }); + const state = makeState({ + data: { schedule: { date: '2047-05-17', groups: [{ time: '9:00 am', sessions: [workshop] }] } }, + }); + + expect(summarize(getFilteredSchedule(state))).toEqual([['9:00 am', ['Ionic React Workshop']]]); + // It is still listed when only one of its tracks is selected. + expect(summarize(getFilteredSchedule({ ...state, data: { ...state.data, filteredTracks: ['React'] } }))).toEqual([ + ['9:00 am', ['Ionic React Workshop']], + ]); + }); + it('leaves out the sessions of excluded tracks and groups left empty', () => { const state = makeState({ data: { filteredTracks: ['Ionic', 'React', 'Tooling'] } }); diff --git a/src/data/selectors.ts b/src/data/selectors.ts index 9c8bbfd0..392db3b5 100644 --- a/src/data/selectors.ts +++ b/src/data/selectors.ts @@ -18,45 +18,24 @@ const getSearchText = (state: AppState) => state.data.searchText; export const getFilteredSchedule = createSelector(getSchedule, getFilteredTracks, (schedule, filteredTracks) => { const groups: ScheduleGroup[] = []; - // Helper function to convert 12-hour time to 24-hour time for proper sorting - const convertTo24Hour = (timeStr: string) => { + // Converts a 12-hour time such as "9:05 am" to minutes since midnight, for sorting by time. + const toMinutes = (timeStr: string) => { const [time, period] = timeStr.toLowerCase().split(' '); - const [parsedHours, minutes] = time.split(':').map(Number); - let hours = parsedHours; - - if (period === 'pm' && hours !== 12) { - hours += 12; - } else if (period === 'am' && hours === 12) { - hours = 0; - } - - return `${hours.toString().padStart(2, '0')}:${minutes || '00'}`; + const [hours, minutes = 0] = time.split(':').map(Number); + const hours24 = (hours % 12) + (period === 'pm' ? 12 : 0); + return hours24 * 60 + minutes; }; // Sort the groups by time - const sortedGroups = [...schedule.groups].sort((a, b) => { - const timeA = convertTo24Hour(a.time); - const timeB = convertTo24Hour(b.time); - return timeA.localeCompare(timeB); - }); + const sortedGroups = [...schedule.groups].sort((a, b) => toMinutes(a.time) - toMinutes(b.time)); sortedGroups.forEach((group: ScheduleGroup) => { - const sessions: Session[] = []; - group.sessions.forEach((session) => { - session.tracks.forEach((track) => { - if (filteredTracks.indexOf(track) > -1) { - sessions.push(session); - } - }); - }); + // A session is listed once, even when more than one of its tracks is selected. + const sessions = group.sessions.filter((session) => session.tracks.some((track) => filteredTracks.includes(track))); if (sessions.length) { // Sort sessions within each group by start time - const sortedSessions = sessions.sort((a, b) => { - const timeA = convertTo24Hour(a.timeStart); - const timeB = convertTo24Hour(b.timeStart); - return timeA.localeCompare(timeB); - }); + const sortedSessions = sessions.sort((a, b) => toMinutes(a.timeStart) - toMinutes(b.timeStart)); const groupToAdd: ScheduleGroup = { time: group.time, diff --git a/src/data/sessions/sessions.reducer.ts b/src/data/sessions/sessions.reducer.ts index 34dcbd61..e2627fec 100644 --- a/src/data/sessions/sessions.reducer.ts +++ b/src/data/sessions/sessions.reducer.ts @@ -10,6 +10,9 @@ export const sessionsReducer = (state: ConfState, action: SessionsActions): Conf return { ...state, ...action.data }; } case 'add-favorite': { + if (state.favorites.includes(action.sessionId)) { + return state; + } return { ...state, favorites: [...state.favorites, action.sessionId] }; } case 'remove-favorite': { diff --git a/src/data/state.test.ts b/src/data/state.test.ts index 6c0e6342..c703efd2 100644 --- a/src/data/state.test.ts +++ b/src/data/state.test.ts @@ -34,6 +34,14 @@ describe('sessions reducer', () => { expect(state.data.favorites).toEqual([2]); }); + it('does not add a favorite twice', () => { + const state = makeState({ data: { favorites: [4] } }); + const next = reducers(state, addFavorite(4)); + + expect(next.data.favorites).toEqual([4]); + expect(next.data).toBe(state.data); + }); + it('removes every copy of a favorite', () => { const state = reducers(makeState({ data: { favorites: [2, 3, 2] } }), removeFavorite(2)); diff --git a/src/data/user/user.actions.ts b/src/data/user/user.actions.ts index 3267ebd6..b9c749a2 100644 --- a/src/data/user/user.actions.ts +++ b/src/data/user/user.actions.ts @@ -24,13 +24,6 @@ export const setData = (data: Partial) => data, }) as const; -export const logoutUser = - () => - async (dispatch: React.Dispatch): Promise => { - await setIsLoggedInData(false); - dispatch(setUsername()); - }; - export const setIsLoggedIn = (loggedIn: boolean) => async (_dispatch: React.Dispatch): Promise<{ readonly type: 'set-is-loggedin'; readonly loggedIn: boolean }> => { diff --git a/src/pages/About.tsx b/src/pages/About.tsx index eb3d0d3c..1954899e 100644 --- a/src/pages/About.tsx +++ b/src/pages/About.tsx @@ -53,7 +53,7 @@ const About: React.FC = () => { - + diff --git a/src/pages/Login.tsx b/src/pages/Login.tsx index 37aa9ea6..6a73149c 100644 --- a/src/pages/Login.tsx +++ b/src/pages/Login.tsx @@ -17,6 +17,7 @@ import React, { useState } from 'react'; import './Login.scss'; import { connect } from '../data/connect'; import { setIsLoggedIn, setUsername } from '../data/user/user.actions'; +import { validationClasses } from '../util/validation'; interface LoginProps { setIsLoggedIn: typeof setIsLoggedIn; @@ -27,6 +28,8 @@ const Login: React.FC = ({ setIsLoggedIn, setUsername: setUsernameAc const router = useIonRouter(); const [login, setLogin] = useState({ username: '', password: '' }); const [submitted, setSubmitted] = useState(false); + // An input shows its error once it has been left, or the form has been submitted. + const [touched, setTouched] = useState({ username: false, password: false }); const onLogin = async (e: React.FormEvent) => { e.preventDefault(); @@ -69,7 +72,9 @@ const Login: React.FC = ({ setIsLoggedIn, setUsername: setUsernameAc type="text" spellCheck={false} autocapitalize="off" - errorText={submitted && !login.username ? 'Username is required' : ''} + className={validationClasses(!!login.username, submitted || touched.username)} + errorText="Username is required" + onIonBlur={() => setTouched((current) => ({ ...current, username: true }))} onIonInput={(e) => setLogin({ ...login, username: e.detail.value! })} required /> @@ -81,7 +86,9 @@ const Login: React.FC = ({ setIsLoggedIn, setUsername: setUsernameAc value={login.password} name="password" type="password" - errorText={submitted && !login.password ? 'Password is required' : ''} + className={validationClasses(!!login.password, submitted || touched.password)} + errorText="Password is required" + onIonBlur={() => setTouched((current) => ({ ...current, password: true }))} onIonInput={(e) => setLogin({ ...login, password: e.detail.value! })} required /> diff --git a/src/pages/SchedulePage.tsx b/src/pages/SchedulePage.tsx index 914e0899..f40fdb59 100644 --- a/src/pages/SchedulePage.tsx +++ b/src/pages/SchedulePage.tsx @@ -88,12 +88,12 @@ const SchedulePage: React.FC = ({ favoritesSchedule, schedule {!ios && !showSearchbar && ( - setShowSearchbar(true)}> + setShowSearchbar(true)}> )} {!showSearchbar && ( - setShowFilterModal(true)}> + setShowFilterModal(true)}> {mode === 'ios' ? 'Filter' : } )} diff --git a/src/pages/SessionDetail.test.tsx b/src/pages/SessionDetail.test.tsx index b420e8c6..f1e6c2c3 100644 --- a/src/pages/SessionDetail.test.tsx +++ b/src/pages/SessionDetail.test.tsx @@ -20,8 +20,11 @@ const renderSession = (id: string, favorites: number[] = []) => { state: makeState({ data: { favorites } }), url: `/tabs/schedule/${id}`, path: '/tabs/schedule/:id' }, ); -// The favorite button is the first icon-only button in the toolbar, before the share button. -const favoriteButton = (container: HTMLElement) => container.querySelector('ion-header ion-button') as HTMLElement; +// The favorite button's label says what pressing it does. +const favoriteButton = (container: HTMLElement) => + container.querySelector('ion-button[aria-label="Favorite"]') as HTMLElement; +const removeFavoriteButton = (container: HTMLElement) => + container.querySelector('ion-button[aria-label="Remove favorite"]') as HTMLElement; describe('SessionDetail', () => { it('shows the session from the URL', () => { @@ -37,19 +40,29 @@ describe('SessionDetail', () => { fireEvent.click(favoriteButton(container)); expect(screen.getByRole('status')).toHaveTextContent(String(ionicTooling.id)); + expect(favoriteButton(container)).toBeNull(); - fireEvent.click(favoriteButton(container)); + fireEvent.click(removeFavoriteButton(container)); expect(screen.getByRole('status')).toBeEmptyDOMElement(); + expect(removeFavoriteButton(container)).toBeNull(); }); it('unfavorites a session that is already a favorite', () => { const { container } = renderSession(String(ionicTooling.id), [ionicTooling.id]); - fireEvent.click(favoriteButton(container)); + fireEvent.click(removeFavoriteButton(container)); expect(screen.getByRole('status')).toBeEmptyDOMElement(); }); + it('labels its icon-only buttons', () => { + const { container } = renderSession(String(ionicTooling.id)); + + expect( + Array.from(container.querySelectorAll('ion-header ion-button'), (button) => button.getAttribute('aria-label')), + ).toEqual(['Favorite', 'Share']); + }); + it('shows a message for an unknown session', () => { renderSession('999'); diff --git a/src/pages/SessionDetail.tsx b/src/pages/SessionDetail.tsx index 2e4cffca..7fe753ce 100644 --- a/src/pages/SessionDetail.tsx +++ b/src/pages/SessionDetail.tsx @@ -61,14 +61,14 @@ const SessionDetail: React.FC = ({ session, addFavorite, rem - toggleFavorite()}> + toggleFavorite()}> {isFavorite ? ( ) : ( )} - shareSession}> + shareSession}> diff --git a/src/pages/Signup.tsx b/src/pages/Signup.tsx index 0932b9ad..5dc546ac 100644 --- a/src/pages/Signup.tsx +++ b/src/pages/Signup.tsx @@ -17,6 +17,7 @@ import React, { useState } from 'react'; import './Signup.scss'; import { connect } from '../data/connect'; import { setIsLoggedIn, setUsername } from '../data/user/user.actions'; +import { validationClasses } from '../util/validation'; interface SignupProps { setIsLoggedIn: typeof setIsLoggedIn; @@ -27,6 +28,8 @@ const Signup: React.FC = ({ setIsLoggedIn, setUsername: setUsername const router = useIonRouter(); const [signup, setSignup] = useState({ username: '', password: '' }); const [submitted, setSubmitted] = useState(false); + // An input shows its error once it has been left, or the form has been submitted. + const [touched, setTouched] = useState({ username: false, password: false }); const onSignup = async (e: React.FormEvent) => { e.preventDefault(); @@ -63,7 +66,9 @@ const Signup: React.FC = ({ setIsLoggedIn, setUsername: setUsername value={signup.username} name="username" type="text" - errorText={submitted && !signup.username ? 'Username is required' : ''} + className={validationClasses(!!signup.username, submitted || touched.username)} + errorText="Username is required" + onIonBlur={() => setTouched((current) => ({ ...current, username: true }))} onIonInput={(e) => setSignup({ ...signup, username: e.detail.value! })} required /> @@ -75,7 +80,9 @@ const Signup: React.FC = ({ setIsLoggedIn, setUsername: setUsername value={signup.password} name="password" type="password" - errorText={submitted && !signup.password ? 'Password is required' : ''} + className={validationClasses(!!signup.password, submitted || touched.password)} + errorText="Password is required" + onIonBlur={() => setTouched((current) => ({ ...current, password: true }))} onIonInput={(e) => setSignup({ ...signup, password: e.detail.value! })} required /> diff --git a/src/pages/SpeakerDetail.tsx b/src/pages/SpeakerDetail.tsx index 00b1eaec..fd5679f9 100644 --- a/src/pages/SpeakerDetail.tsx +++ b/src/pages/SpeakerDetail.tsx @@ -108,10 +108,10 @@ const SpeakerDetail: React.FC = ({ speaker }) => { - openContact(speaker)}> + openContact(speaker)}> - openSpeakerShare(speaker)}> + openSpeakerShare(speaker)}> diff --git a/src/pages/Support.tsx b/src/pages/Support.tsx index 623e4a04..27febdd5 100644 --- a/src/pages/Support.tsx +++ b/src/pages/Support.tsx @@ -14,12 +14,15 @@ import { useIonViewWillEnter, } from '@ionic/react'; import React, { useState } from 'react'; + import './Support.scss'; +import { validationClasses } from '../util/validation'; const Support: React.FC = () => { - const [present] = useIonToast(); + const [present, dismiss] = useIonToast(); const [supportMessage, setSupportMessage] = useState(''); const [submitted, setSubmitted] = useState(false); + const [touched, setTouched] = useState(false); useIonViewWillEnter(() => { present({ @@ -35,7 +38,11 @@ const Support: React.FC = () => { if (supportMessage) { setSupportMessage(''); setSubmitted(false); + setTouched(false); + // The hook only shows one toast at a time and ignores new ones while one is open, such as the notice + // shown when the page opens, so close that first. + await dismiss(); present({ message: 'Your support request has been sent.', duration: 3000, @@ -67,7 +74,9 @@ const Support: React.FC = () => { value={supportMessage} name="supportQuestion" rows={6} - errorText={submitted && !supportMessage ? 'Support message is required' : ''} + className={validationClasses(!!supportMessage, submitted || touched)} + errorText="Support message is required" + onIonBlur={() => setTouched(true)} onIonInput={(e) => setSupportMessage(e.detail.value!)} required /> diff --git a/src/pages/requiredFields.test.tsx b/src/pages/requiredFields.test.tsx new file mode 100644 index 00000000..fa4c9208 --- /dev/null +++ b/src/pages/requiredFields.test.tsx @@ -0,0 +1,50 @@ +import { fireEvent } from '@testing-library/react'; +import type React from 'react'; +import { describe, expect, it } from 'vitest'; + +import { makeState } from '../testing/fixtures'; +import { renderWithState } from '../testing/render'; +import { validationClasses } from '../util/validation'; + +import Login from './Login'; +import Signup from './Signup'; +import Support from './Support'; + +// Ionic only shows an input's error text when it has both the ion-invalid and ion-touched classes. +// Vitest loads the server build of Ionic's React wrappers, which doesn't pass props or events to the inputs, so +// typing and leaving an input are covered by the e2e tests. +const showsError = (input: Element) => + input.classList.contains('ion-invalid') && input.classList.contains('ion-touched'); + +const fields = (container: HTMLElement) => Array.from(container.querySelectorAll('ion-input, ion-textarea')); + +describe('validationClasses', () => { + it('marks a field valid or invalid, and touched or untouched', () => { + expect(validationClasses(true, false)).toBe('ion-valid ion-untouched'); + expect(validationClasses(false, true)).toBe('ion-invalid ion-touched'); + }); +}); + +describe.each<[string, React.ReactElement, number]>([ + ['Login', , 2], + ['Signup', , 2], + ['Support', , 1], +])('%s required fields', (_name, page, count) => { + const renderPage = () => renderWithState(page, { state: makeState() }); + + it('do not show an error before they are used', () => { + const { container } = renderPage(); + + expect(fields(container)).toHaveLength(count); + expect(fields(container).some(showsError)).toBe(false); + }); + + it('show an error on every empty field when the form is submitted', () => { + const { container } = renderPage(); + + fireEvent.submit(container.querySelector('form')!); + + expect(fields(container)).toHaveLength(count); + expect(fields(container).every(showsError)).toBe(true); + }); +}); diff --git a/src/util/validation.ts b/src/util/validation.ts new file mode 100644 index 00000000..6f324ba5 --- /dev/null +++ b/src/util/validation.ts @@ -0,0 +1,6 @@ +/** + * The classes Ionic's inputs need to show their error text: it only shows on an input that is both + * `ion-invalid` and `ion-touched`. Angular's forms module adds these for us, but in React we add them ourselves. + */ +export const validationClasses = (valid: boolean, touched: boolean): string => + `${valid ? 'ion-valid' : 'ion-invalid'} ${touched ? 'ion-touched' : 'ion-untouched'}`;