Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 15 additions & 6 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
import { PeoplePage } from './components/PeoplePage';
import { Navbar } from './components/Navbar';

import './App.scss';

import { PeoplePage } from './pages/PeoplePage';
import { Navbar } from './components/Navbar';
import { HomePage } from './pages/HomePage';
import { Navigate, Route, Routes } from 'react-router-dom';
import { NotFoundPage } from './pages/NotFoundPage';

export const App = () => {
return (
<div data-cy="app">
<Navbar />

<div className="section">
<div className="container">

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link navigates to /people/${person.slug} without preserving search params. According to checklist item #3, search params must be preserved when navigating within the People page (selecting a person). Use SearchLink component with preserved params instead.

<h1 className="title">Home Page</h1>
<h1 className="title">Page not found</h1>
<PeoplePage />
<Routes>
<Route path="/" element={<HomePage />} />
<Route path="/home" element={<Navigate to="/" replace />} />

<Route path="/people" element={<PeoplePage />} />
<Route path="/people/:slug" element={<PeoplePage />} />

<Route path="*" element={<NotFoundPage />} />
</Routes>
</div>
</div>
</div>
Expand Down
7 changes: 4 additions & 3 deletions src/components/Loader/Loader.scss
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
.Loader {
display: flex;
width: 100%;
justify-content: center;
align-items: center;
justify-content: center;
width: 100%;

&__content {
border-radius: 50%;
width: 2em;
height: 2em;
margin: 1em auto;
border: 0.3em solid #ddd;
border-left-color: #000;
border-radius: 50%;

animation: load8 1.2s infinite linear;
}
}
Expand Down
26 changes: 20 additions & 6 deletions src/components/Navbar.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { NavLink } from 'react-router-dom';
import classNames from 'classnames';

export const Navbar = () => {
return (
<nav
Expand All @@ -8,17 +11,28 @@ export const Navbar = () => {
>
<div className="container">
<div className="navbar-brand">
<a className="navbar-item" href="#/">
<NavLink
to="/"
className={({ isActive }) =>
classNames('navbar-item', {
'has-background-grey-lighter': isActive,
})
}
Comment on lines +17 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The link uses a hardcoded path /people/${person.slug} without preserving search params. When navigating to a person's details, the user's active filters and sort settings are lost. Use the SearchLink component or include the current search params in the URL to preserve the filter/sort state as required by checklist item #3.

>
Home
</a>
</NavLink>

<a
<NavLink
aria-current="page"
className="navbar-item has-background-grey-lighter"
href="#/people"
to="/people"
className={({ isActive }) =>
classNames('navbar-item', {
'has-background-grey-lighter': isActive,
})
}
>
People
</a>
</NavLink>
</div>
</div>
</nav>
Expand Down
110 changes: 94 additions & 16 deletions src/components/PeopleFilters.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,89 @@
import { useSearchParams } from 'react-router-dom';

export const PeopleFilters = () => {
const [searchParams, setSearchParams] = useSearchParams();
const selectedCenturies = searchParams.getAll('centuries');

const sex = searchParams.get('sex') || 'all';

const setSex = (value: string) => {
const params = new URLSearchParams(searchParams);

if (value === 'all') {
params.delete('sex');
} else {
params.set('sex', value);
}

setSearchParams(params);
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This navigation doesn't preserve search params. Per requirement #2, clicking on a person should keep the current filters/sort active. Use the SearchLink component or construct a link with the current search params included.


const handleQueryChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const params = new URLSearchParams(searchParams);

const value = e.target.value;

if (value) {
params.set('query', value);
} else {
params.delete('query');
}

setSearchParams(params);
};

const toggleCentury = (value: number) => {
Comment on lines +29 to +35

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical bug: When params.toString() is empty, this returns ? creating malformed URLs like ?sort=name&order=asc or ?query=mae?sort=name (double ?). Should return . when params are empty to avoid this issue.

const params = new URLSearchParams(searchParams);
const current = params.getAll('centuries');
const strValue = String(value);

if (current.includes(strValue)) {
const updated = current.filter(c => c !== strValue);

params.delete('centuries');
updated.forEach(c => params.append('centuries', c));
} else {
params.append('centuries', strValue);
}

setSearchParams(params);
};

const isSelected = (value: number) =>
selectedCenturies.includes(String(value));

const resetCenturies = () => {
const params = new URLSearchParams(searchParams);

params.delete('centuries');
setSearchParams(params);
};

const resetAllFilters = () => {
setSearchParams(new URLSearchParams());
};

return (
<nav className="panel">
<p className="panel-heading">Filters</p>

<p className="panel-tabs" data-cy="SexFilter">
<a className="is-active" href="#/people">
<a
className={sex === 'all' ? 'is-active' : ''}
onClick={() => setSex('all')}
>
All
</a>
<a className="" href="#/people?sex=m">
<a
className={sex === 'm' ? 'is-active' : ''}
onClick={() => setSex('m')}
>
Male
</a>
<a className="" href="#/people?sex=f">
<a
className={sex === 'f' ? 'is-active' : ''}
onClick={() => setSex('f')}
>
Female
</a>
</p>
Expand All @@ -22,6 +95,8 @@ export const PeopleFilters = () => {
type="search"
className="input"
placeholder="Search"
value={searchParams.get('query') || ''}
onChange={handleQueryChange}
/>

<span className="icon is-left">
Expand All @@ -35,40 +110,40 @@ export const PeopleFilters = () => {
<div className="level-left">
<a
data-cy="century"
className="button mr-1"
href="#/people?centuries=16"
className={`button mr-1 ${isSelected(16) ? 'is-info' : ''}`}
onClick={() => toggleCentury(16)}
>
16
</a>

<a
data-cy="century"
className="button mr-1 is-info"
href="#/people?centuries=17"
className={`button mr-1 ${isSelected(17) ? 'is-info' : ''}`}
onClick={() => toggleCentury(17)}
>
17
</a>

<a
data-cy="century"
className="button mr-1 is-info"
href="#/people?centuries=18"
className={`button mr-1 ${isSelected(18) ? 'is-info' : ''}`}
onClick={() => toggleCentury(18)}
>
18
</a>

<a
data-cy="century"
className="button mr-1 is-info"
href="#/people?centuries=19"
className={`button mr-1 ${isSelected(19) ? 'is-info' : ''}`}
onClick={() => toggleCentury(19)}
>
19
</a>

<a
data-cy="century"
className="button mr-1"
href="#/people?centuries=20"
className={`button mr-1 ${isSelected(20) ? 'is-info' : ''}`}
onClick={() => toggleCentury(20)}
>
20
</a>
Expand All @@ -77,8 +152,8 @@ export const PeopleFilters = () => {
<div className="level-right ml-4">
<a
data-cy="centuryALL"
className="button is-success is-outlined"
href="#/people"
className={`button is-success ${selectedCenturies.length !== 0 ? 'is-outlined' : ''}`}
onClick={resetCenturies}
>
All
</a>
Expand All @@ -87,7 +162,10 @@ export const PeopleFilters = () => {
</div>

<div className="panel-block">
<a className="button is-link is-outlined is-fullwidth" href="#/people">
<a
className="button is-link is-outlined is-fullwidth"
onClick={resetAllFilters}
>
Reset all filters
</a>
</div>
Expand Down
33 changes: 0 additions & 33 deletions src/components/PeoplePage.tsx

This file was deleted.

Loading
Loading