Skip to content

Commit

Permalink
Simplify how we pass the props to CompareWithBase (mozilla#577)
Browse files Browse the repository at this point in the history
  • Loading branch information
julienw authored Nov 22, 2023
2 parents 08d2c3d + 11c192d commit 817529b
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 130 deletions.
35 changes: 12 additions & 23 deletions src/__tests__/Search/CompareWithBase.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import CompareWithBase from '../../components/Search/CompareWithBase';
import SearchView from '../../components/Search/SearchView';
import { Strings } from '../../resources/Strings';
import useProtocolTheme from '../../theme/protocolTheme';
import type { Repository } from '../../types/state';
import getTestData from '../utils/fixtures';
import { renderWithRouter, store } from '../utils/setupTests';
import { screen } from '../utils/test-utils';
Expand All @@ -18,34 +17,24 @@ const protocolTheme = renderHook(() => useProtocolTheme()).result.current
const themeMode = protocolTheme.palette.mode;
function renderComponent(isEditable: boolean) {
const { testData } = getTestData();
const selectedRevsBase = testData.slice(0, 1);
const selectedRevsNew = testData.slice(1, 3);
const baseRevs = testData.slice(0, 1);
const newRevs = testData.slice(1, 3);

const displayedCheckedRevisions = {
baseRevs: selectedRevsBase,
newRevs: selectedRevsNew,
};

const baseRepos = selectedRevsBase.map((item) => {
const selectedRep = repoMap[item.repository_id];
return selectedRep;
});

const newRepos = selectedRevsNew.map((item) => {
const selectedRep = repoMap[item.repository_id];
return selectedRep;
});
// The "??" operations below are so that Typescript doesn't wonder about the
// undefined value later.
const baseRepos = baseRevs.map(
(item) => repoMap[item.repository_id] ?? 'try',
);
const newRepos = newRevs.map((item) => repoMap[item.repository_id] ?? 'try');

const displayedRepositories = {
baseRepos: baseRepos as Repository['name'][],
newRepos: newRepos as Repository['name'][],
};
renderWithRouter(
<CompareWithBase
isEditable={isEditable}
mode={themeMode}
displayedRevisions={displayedCheckedRevisions}
displayedRepositories={displayedRepositories}
baseRevs={baseRevs}
newRevs={newRevs}
baseRepos={baseRepos}
newRepos={newRepos}
/>,
);
}
Expand Down
55 changes: 35 additions & 20 deletions src/__tests__/Search/SearchView.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import userEvent from '@testing-library/user-event';
import { act } from 'react-dom/test-utils';

import SearchView from '../../components/Search/SearchView';
import { setSelectedRevisions } from '../../reducers/SelectedRevisionsSlice';
import { Strings } from '../../resources/Strings';
import useProtocolTheme from '../../theme/protocolTheme';
import { RevisionsList, InputType } from '../../types/state';
Expand All @@ -17,7 +16,7 @@ const protocolTheme = renderHook(() => useProtocolTheme()).result.current
const toggleColorMode = renderHook(() => useProtocolTheme()).result.current
.toggleColorMode;
function renderComponent() {
renderWithRouter(
return renderWithRouter(
<SearchView
toggleColorMode={toggleColorMode}
protocolTheme={protocolTheme}
Expand Down Expand Up @@ -150,15 +149,13 @@ describe('Base Search', () => {
const searchInput = screen.getAllByRole('textbox')[0];
await user.click(searchInput);

await act(async () => {
expect(store.getState().search[searchType].searchResults).toStrictEqual(
testData,
);
});

const comment = await screen.findAllByText("you've got no arms left!");
expect(comment[0]).toBeInTheDocument();

expect(store.getState().search[searchType].searchResults).toStrictEqual(
testData,
);

// Press Escape key to hide search results.

await user.keyboard('{Escape}');
Expand Down Expand Up @@ -279,23 +276,41 @@ describe('Base Search', () => {

it('should have compare button and once clicked should redirect to results page with the right query params', async () => {
const { testData } = getTestData();
fetchTestData(testData);

const { history } = renderWithRouter(
<SearchView
toggleColorMode={toggleColorMode}
protocolTheme={protocolTheme}
title={Strings.metaData.pageTitle.search}
/>,
);
const { history } = renderComponent();
expect(history.location.pathname).toEqual('/');

const user = userEvent.setup({ delay: null });
act(() => {
store.dispatch(
setSelectedRevisions({ selectedRevisions: testData.slice(0, 2) }),
);
});

// focus first input to show results
const inputs = screen.getAllByRole('textbox');
await user.click(inputs[0]);

// Select a base rev
let items = await screen.findAllByText("you've got no arms left!");
await user.click(items[0]);

// Press Escape key to hide search results.
await user.keyboard('{Escape}');
expect(items[0]).not.toBeInTheDocument();

// Check that the item has been added
await screen.findByText(/no arms left/);

// Now focus the second input
await user.click(inputs[1]);
// Select a new rev
items = await screen.findAllByText("it's just a flesh wound");
await user.click(items[0]);

// Press Escape key to hide search results.
await user.keyboard('{Escape}');

// Check that the item has been added
await screen.findByText(/flesh wound/);

// Press the compare button
const compareButton = document.querySelector('.compare-button');
await user.click(compareButton as HTMLElement);

Expand Down
33 changes: 12 additions & 21 deletions src/components/CompareResults/ResultsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,14 @@ function ResultsView(props: ResultsViewProps) {
(state) => state.selectedRevisions.new,
);

const displayedSelectedRevisions = {
baseRevs: selectedRevisionsListBase,
newRevs: selectedRevisionsListNew,
};

const selectedBaseRepositories = selectedRevisionsListBase.map((item) => {
const selectedRep = repoMap[item.repository_id];
return selectedRep;
});

const selectedNewRepositories = selectedRevisionsListNew.map((item) => {
const selectedRep = repoMap[item.repository_id];
return selectedRep;
});

const displayedRepositories = {
baseRepos: selectedBaseRepositories as Repository['name'][],
newRepos: selectedNewRepositories as Repository['name'][],
};
// The "??" operations below are so that Typescript doesn't wonder about the
// undefined value later.
const selectedBaseRepositories = selectedRevisionsListBase.map(
(item) => repoMap[item.repository_id] ?? 'try',
);
const selectedNewRepositories = selectedRevisionsListNew.map(
(item) => repoMap[item.repository_id] ?? 'try',
);

const { dispatchFetchCompareResults, dispatchFakeCompareResults } =
useFetchCompareResults();
Expand Down Expand Up @@ -150,8 +139,10 @@ function ResultsView(props: ResultsViewProps) {
<CompareWithBase
mode={themeMode}
isEditable={true}
displayedRevisions={displayedSelectedRevisions}
displayedRepositories={displayedRepositories}
baseRevs={selectedRevisionsListBase}
newRevs={selectedRevisionsListNew}
baseRepos={selectedBaseRepositories}
newRepos={selectedNewRepositories}
/>
</section>
<Grid container alignItems='center' justifyContent='center'>
Expand Down
64 changes: 19 additions & 45 deletions src/components/Search/CompareWithBase.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState, useMemo } from 'react';
import { useState } from 'react';

import Divider from '@mui/material/Divider';
import Grid from '@mui/material/Grid';
Expand All @@ -8,12 +8,7 @@ import { useAppSelector } from '../../hooks/app';
import { Strings } from '../../resources/Strings';
import { CompareCardsStyles } from '../../styles';
import { SearchStyles } from '../../styles';
import type {
ThemeMode,
RevisionsList,
Repository,
InputType,
} from '../../types/state';
import type { ThemeMode, RevisionsList, Repository } from '../../types/state';
import CompareButton from './CompareButton';
import FrameworkDropdown from './FrameworkDropdown';
import SearchComponent from './SearchComponent';
Expand All @@ -25,21 +20,19 @@ const stringsNew = Strings.components.searchDefault.base.collapsed.revision;
interface CompareWithBaseProps {
mode: ThemeMode;
isEditable: boolean;
displayedRevisions: {
baseRevs: RevisionsList[];
newRevs: RevisionsList[];
};
displayedRepositories: {
baseRepos: Repository['name'][];
newRepos: Repository['name'][];
};
baseRevs: RevisionsList[];
newRevs: RevisionsList[];
baseRepos: Repository['name'][];
newRepos: Repository['name'][];
}

function CompareWithBase({
mode,
isEditable,
displayedRevisions,
displayedRepositories,
baseRevs,
newRevs,
baseRepos,
newRepos,
}: CompareWithBaseProps) {
const [expanded, setExpanded] = useState(true);

Expand All @@ -48,31 +41,6 @@ function CompareWithBase({
const search = useAppSelector((state) => state.search);
const baseRepository = search.base.repository;
const newRepository = search.new.repository;
const revisionsAndReposBase = useMemo(() => {
const revsAndRepos = {
revisions: displayedRevisions.baseRevs,
repositories: displayedRepositories.baseRepos,
};
return revsAndRepos;
}, [displayedRevisions.baseRevs, displayedRepositories.baseRepos]);

const revisionsAndReposNew = useMemo(() => {
const revsAndRepos = {
revisions: displayedRevisions.newRevs,
repositories: displayedRepositories.newRepos,
};
return revsAndRepos;
}, [displayedRevisions.baseRevs, displayedRepositories.baseRepos]);
const baseSearchProps = {
...stringsBase,
...revisionsAndReposBase,
searchType: 'base' as InputType,
};
const newSearchProps = {
...stringsNew,
...revisionsAndReposNew,
searchType: 'new' as InputType,
};
const isWarning =
(baseRepository === 'try' && newRepository !== 'try') ||
(baseRepository !== 'try' && newRepository === 'try');
Expand Down Expand Up @@ -116,16 +84,22 @@ function CompareWithBase({
<Divider className='divider' />
<div className='form-wrapper'>
<SearchComponent
searchType='base'
isEditable={isEditable}
isWarning={isWarning}
mode={mode}
{...baseSearchProps}
revisions={baseRevs}
repositories={baseRepos}
{...stringsBase}
/>
<SearchComponent
searchType='new'
isEditable={isEditable}
mode={mode}
isWarning={isWarning}
{...newSearchProps}
mode={mode}
revisions={newRevs}
repositories={newRepos}
{...stringsNew}
/>
<Grid
item
Expand Down
34 changes: 13 additions & 21 deletions src/components/Search/SearchContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { repoMap, searchView } from '../../common/constants';
import { useAppSelector } from '../../hooks/app';
import { Strings } from '../../resources/Strings';
import { SearchContainerStyles } from '../../styles';
import type { ThemeMode, Repository } from '../../types/state';
import type { ThemeMode } from '../../types/state';
import CompareOverTime from './CompareOverTime';
import CompareWithBase from './CompareWithBase';

Expand All @@ -21,25 +21,15 @@ function SearchContainer(props: SearchViewProps) {
const checkedRevisionsListBase = useAppSelector(
(state) => state.search.base.checkedRevisions,
);
const displayedCheckedRevisions = {
baseRevs: checkedRevisionsListBase,
newRevs: checkedRevisionsListNew,
};

const checkedNewRepos = checkedRevisionsListNew.map((item) => {
const selectedRep = repoMap[item.repository_id];
return selectedRep;
});

const checkedBaseRepos = checkedRevisionsListBase.map((item) => {
const selectedRep = repoMap[item.repository_id];
return selectedRep;
});

const displayedRepositories = {
baseRepos: checkedBaseRepos as Repository['name'][],
newRepos: checkedNewRepos as Repository['name'][],
};
// The "??" operations below are so that Typescript doesn't wonder about the
// undefined value later.
const checkedNewRepos = checkedRevisionsListNew.map(
(item) => repoMap[item.repository_id] ?? 'try',
);
const checkedBaseRepos = checkedRevisionsListBase.map(
(item) => repoMap[item.repository_id] ?? 'try',
);

return (
<section
Expand All @@ -51,8 +41,10 @@ function SearchContainer(props: SearchViewProps) {
<CompareWithBase
mode={themeMode}
isEditable={false}
displayedRevisions={displayedCheckedRevisions}
displayedRepositories={displayedRepositories}
baseRevs={checkedRevisionsListBase}
newRevs={checkedRevisionsListNew}
baseRepos={checkedBaseRepos}
newRepos={checkedNewRepos}
/>
{/* hidden until post-mvp release */}
<CompareOverTime mode={themeMode} />
Expand Down

0 comments on commit 817529b

Please sign in to comment.