Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enhancement: Make results tiltle editable and URL persistent #774

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
bb6ce4f
refactor: replace text with input
FatumaA Oct 9, 2024
a62c560
fix: Sync URL state and input value
FatumaA Oct 9, 2024
327c03a
fix: order imports correctly
FatumaA Oct 9, 2024
1385406
style: Mirror original title styles
FatumaA Oct 9, 2024
282252e
fix: persist the URL state and title when page is refreshed
FatumaA Oct 10, 2024
8093d55
style: remove unused class and its definition
FatumaA Oct 10, 2024
0c7a46d
fix: a11y add hidden label text to input
FatumaA Oct 10, 2024
f31e15d
test: update the resultsView corresponding test
FatumaA Oct 10, 2024
3acd065
test: update snapshots
FatumaA Oct 10, 2024
5c96760
fix: lint
FatumaA Oct 10, 2024
5623b97
feat: extract logic into component
FatumaA Oct 14, 2024
447de2d
chore: clean up results main and use new component
FatumaA Oct 14, 2024
e9483d6
refactor: prefer custom search param hook
FatumaA Oct 14, 2024
bfabdad
feat: toggle between the value of label and input
FatumaA Oct 14, 2024
52a16a1
fix: switch to label on blur too
FatumaA Oct 14, 2024
8e75e50
fix: protect against empty strings
FatumaA Oct 14, 2024
694e07e
test: add testable data attribute to label
FatumaA Oct 14, 2024
f33e4ee
test: update test to look for label data testid attribute
FatumaA Oct 14, 2024
603be9d
test: add draft test for results title component
FatumaA Oct 14, 2024
ffddac0
chore: remove timeout
FatumaA Oct 14, 2024
8d4f891
fix: use ternary for entire component
FatumaA Oct 22, 2024
f0168bc
chore: add wrapper div for testing
FatumaA Oct 22, 2024
1633d54
test: update test definition in ResultsView.test
FatumaA Oct 22, 2024
6ca4055
fix: a11y - add a label on input for screen readers
FatumaA Oct 22, 2024
8fa4b94
style: add class to span containing the title
FatumaA Oct 22, 2024
2b3c4aa
refactor: use existing button
FatumaA Oct 22, 2024
4bf4ef4
chore: remove unused style definition
FatumaA Oct 22, 2024
1b2bc64
chore: remove unused id and class on edit button
FatumaA Oct 22, 2024
2696f6c
fix: wrap the edit button in MUI's icon button component
FatumaA Oct 22, 2024
cdd275f
style: make text and icon centered
FatumaA Oct 22, 2024
f5588d2
fix: remove button in button with icon image
FatumaA Oct 22, 2024
1613663
fix: pass the correct theme mode to ResultsTitle component
FatumaA Oct 22, 2024
d613e18
fix: handle escape when editing title
FatumaA Oct 22, 2024
6add9ba
fix: a11y - add focus to input when in edit mode
FatumaA Oct 22, 2024
5d801c1
test: update tests for ResultsTitle component
FatumaA Oct 22, 2024
7733853
test: update snapshots in ResultsTable and CompareWithBase tests
FatumaA Oct 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions src/__tests__/CompareResults/ResultsTitle.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { render, screen } from '@testing-library/react';
import userEvent, { UserEvent } from '@testing-library/user-event';

import { ResultsTitle } from '../../components/CompareResults/ResultsTitle';
import useRawSearchParams from '../../hooks/useRawSearchParams';

jest.mock('../../hooks/useRawSearchParams');

let user: UserEvent;

const setupTest = (initialParams = new URLSearchParams()) => {
const updateSearchParams = jest.fn();
(useRawSearchParams as jest.Mock).mockReturnValue([
initialParams,
updateSearchParams,
]);
return { updateSearchParams };
};

describe('Results Title', () => {
beforeEach(() => {
user = userEvent.setup({ delay: null });
});

test.each([
['default title', new URLSearchParams(), 'Results'],
['custom title', new URLSearchParams('title=Custom+Title'), 'Custom Title'],
])('renders spaces correctly', (_, params, expected) => {
setupTest(params);
render(<ResultsTitle mode={'light'} />);
expect(screen.getByText(expected)).toBeInTheDocument();
});

it('enables editing when edit icon is clicked', async () => {
setupTest();
render(<ResultsTitle mode={'light'} />);

const editIcon = screen.getByRole('button', { name: 'edit the title' });
await user.click(editIcon);

const inputField = screen.getByRole('textbox');
expect(inputField).toBeInTheDocument();
expect(inputField).toHaveValue('Results');
});

it('updates title and URL with new value', async () => {
const { updateSearchParams } = setupTest();
render(<ResultsTitle mode={'light'} />);

await user.click(screen.getByRole('button', { name: 'edit the title' }));
const inputField = screen.getByRole('textbox');

await user.clear(inputField);
await user.type(inputField, 'New Title');
await user.keyboard('{Enter}');

expect(screen.getByTestId('results-title-component')).toHaveTextContent(
'NewTitle',
);

const updatedParams = updateSearchParams.mock
.calls[0][0] as URLSearchParams;

expect(updatedParams.get('title')).toBe('NewTitle');
expect(updateSearchParams).toHaveBeenCalledWith(
expect.any(URLSearchParams),
{ method: 'push' },
);
});

it('reverts to original title on escape', async () => {
setupTest(new URLSearchParams('title=Original+Title'));
render(<ResultsTitle mode='light' />);

await user.click(screen.getByRole('button', { name: 'edit the title' }));
const inputField = screen.getByRole('textbox');

await user.clear(inputField);
await user.type(inputField, 'New Title');
await user.keyboard('{Escape}');

expect(screen.getByText('Original Title')).toBeInTheDocument();
});
});
2 changes: 1 addition & 1 deletion src/__tests__/CompareResults/ResultsView.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('Results View', () => {
const user = userEvent.setup({ delay: null });
renderWithRoute(<ResultsView title={Strings.metaData.pageTitle.results} />);

const header = await screen.findByText('Results');
const header = await screen.findByTestId('results-title-component');

expect(header).toBeInTheDocument();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -690,9 +690,35 @@ exports[`Results Table Should match snapshot 1`] = `
class="MuiGrid-root MuiGrid-container fau5w0k css-jryjzr-MuiGrid-root"
>
<div
class="MuiGrid-root MuiGrid-item f1q8wx css-j8gwlw-MuiGrid-root"
class="MuiGrid-root MuiGrid-item css-j8gwlw-MuiGrid-root"
>
Results
<div
data-testid="results-title-component"
>
<div
class="f1v35uxs"
>
<span
class="f1q8wx"
>
Results
</span>
<button
aria-label="edit the title"
class="MuiButtonBase-root MuiIconButton-root MuiIconButton-sizeMedium css-1qlpk5e-MuiButtonBase-root-MuiIconButton-root"
tabindex="0"
type="button"
>
<img
alt="edit-icon"
src="pencil.svg"
/>
<span
class="MuiTouchRipple-root css-8je8zh-MuiTouchRipple-root"
/>
</button>
</div>
</div>
</div>
<div
class="MuiGrid-root MuiGrid-item f1it3pp1 css-j8gwlw-MuiGrid-root"
Expand Down
13 changes: 4 additions & 9 deletions src/components/CompareResults/ResultsMain.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { truncateHash } from '../../utils/helpers';
import type { LoaderReturnValue } from './loader';
import type { LoaderReturnValue as OverTimeLoaderReturnValue } from './overTimeLoader';
import ResultsTable from './ResultsTable';
import { ResultsTitle } from './ResultsTitle';

function getPunctuationMark(index: number, newRevs: string[]) {
return index != newRevs.length - 1 ? ', ' : '.';
Expand All @@ -36,6 +37,7 @@ function ResultsMain() {
const { view } = useLoaderData() as
| LoaderReturnValue
| OverTimeLoaderReturnValue;

const styles = {
alert: style({
width: '100%',
Expand All @@ -45,12 +47,6 @@ function ResultsMain() {
margin: '0 auto',
marginBottom: '80px',
}),
title: style({
...FontsRaw.HeadingXS,
fontWeight: 700,
letterSpacing: '-0.01em',
margin: 0,
}),
subtitle: style({
...FontsRaw.BodyDefault,
fontSize: '15px',
Expand Down Expand Up @@ -96,8 +92,8 @@ function ResultsMain() {
<Container className={styles.container} data-testid='results-main'>
<header>
<Grid container className={styles.titleContainer} component='h2'>
<Grid item className={styles.title}>
Results
<Grid item>
<ResultsTitle mode={themeMode} />
</Grid>
<Grid item className={styles.subtitle}>
{subtitles[view]}
Expand All @@ -121,5 +117,4 @@ function ResultsMain() {
</Container>
);
}

export default ResultsMain;
120 changes: 120 additions & 0 deletions src/components/CompareResults/ResultsTitle.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import { useEffect, useRef, useState } from 'react';

import { IconButton, Input } from '@mui/material';
import { style } from 'typestyle';

import useRawSearchParams from '../../hooks/useRawSearchParams';
import { FontsRaw } from '../../styles';
import pencilDark from '../../theme/img/pencil-dark.svg';
import pencil from '../../theme/img/pencil.svg';

interface EditButtonProps {
mode: string;
}

export const ResultsTitle = ({ mode }: EditButtonProps) => {
const [searchParams, updateSearchParams] = useRawSearchParams();
const [resultsTitle, setResultsTitle] = useState(
searchParams.get('title') || 'Results',
);
const [isEditing, setIsEditing] = useState(false);
const inputRef = useRef<HTMLInputElement>(null);

const styles = {
title: style({
FatumaA marked this conversation as resolved.
Show resolved Hide resolved
...FontsRaw.HeadingXS,
fontWeight: 700,
letterSpacing: '-0.01em',
margin: 0,
FatumaA marked this conversation as resolved.
Show resolved Hide resolved
}),
titleWrapper: style({
alignItems: 'center',
display: 'flex',
gap: '10px',
}),
screenReaderOnly: style({
position: 'absolute',
left: '-9999px',
overflow: 'hidden',
whiteSpace: 'nowrap',
}),
};

const handleResultsChange = (event: React.ChangeEvent<HTMLInputElement>) => {
const newTitle = event.target.value.trim();
setResultsTitle(newTitle);
};

const handleEdit = () => setIsEditing(true);
FatumaA marked this conversation as resolved.
Show resolved Hide resolved

const handleEditComplete = (
event: React.FocusEvent | React.KeyboardEvent,
) => {
if ((event as React.KeyboardEvent).key === 'Escape') {
const title = searchParams.get('title');
setResultsTitle(title || 'Results');
setIsEditing(false);
return;
}

const isDoneEditing =
(event as React.KeyboardEvent).key === 'Enter' || event.type === 'blur';

if (isDoneEditing) {
const safeTitle = resultsTitle || 'Results';
setResultsTitle(safeTitle);

const newSearchParams = new URLSearchParams(searchParams);
newSearchParams.set('title', safeTitle);
updateSearchParams(newSearchParams, {
method: 'push',
});

setIsEditing(false);
}
};

const buttonIcon = (
<img
src={mode === 'light' ? pencil.toString() : pencilDark.toString()}
alt='edit-icon'
/>
);

useEffect(() => {
if (isEditing && inputRef.current) {
inputRef.current.focus();
inputRef.current.select();
}
}, [isEditing]);

return (
<div data-testid='results-title-component'>
{isEditing ? (
<>
<label htmlFor='results' className={styles.screenReaderOnly}>
Results Title
</label>
<Input
id='results'
type='text'
inputRef={inputRef}
name='results-title'
value={resultsTitle}
onChange={handleResultsChange}
onBlur={handleEditComplete}
onKeyDown={handleEditComplete}
className={styles.title}
/>
</>
) : (
<div className={styles.titleWrapper}>
<span className={styles.title}>{resultsTitle}</span>
<IconButton onClick={handleEdit} aria-label='edit the title'>
{buttonIcon}
</IconButton>
</div>
)}
</div>
);
};