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
7 changes: 3 additions & 4 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { PeoplePage } from './components/PeoplePage';
// import { PeoplePage } from './components/PeoplePage';
import { Navbar } from './components/Navbar';

import './App.scss';
import { Outlet } from 'react-router-dom';

export const App = () => {
return (
Expand All @@ -10,9 +11,7 @@ export const App = () => {

<div className="section">
<div className="container">
<h1 className="title">Home Page</h1>
<h1 className="title">Page not found</h1>
<PeoplePage />
<Outlet />
</div>
</div>
</div>
Expand Down
23 changes: 23 additions & 0 deletions src/Root.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import {
Navigate,
Route,
HashRouter as Router,
Routes,
} from 'react-router-dom';
import { App } from './App';
import { PageNotFound } from './components/Pages/PageNotFound';
import { HomePage } from './components/Pages/HomePage';
import { PeoplePage } from './components/PeoplePage';

export const Root = () => (
<Router>
<Routes>
<Route path="/" element={<App />}>
<Route path="*" element={<PageNotFound />} />
<Route path="home" element={<Navigate to="/" replace />} />
<Route index element={<HomePage />} />
<Route path="people/:peopleSlug?" element={<PeoplePage />} />
</Route>
</Routes>
</Router>
);
30 changes: 23 additions & 7 deletions src/components/Navbar.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import '../App.scss';
import { NavLink, useLocation } from 'react-router-dom';
import classNames from 'classnames';

export const Navbar = () => {
const location = useLocation();

return (
<nav
data-cy="nav"
Expand All @@ -8,17 +14,27 @@ export const Navbar = () => {
>
<div className="container">
<div className="navbar-brand">
<a className="navbar-item" href="#/">
<NavLink
className={({ isActive }) => {
return classNames('navbar-item', {
'has-background-grey-lighter': isActive,
});
}}
to="/"
>
Home
</a>
</NavLink>

<a
aria-current="page"
className="navbar-item has-background-grey-lighter"
href="#/people"
<NavLink
className={({ isActive }) => {
return classNames('navbar-item', {
'has-background-grey-lighter': isActive,
});
}}
to={{ pathname: '/people', search: location.search }}
>
People
</a>
</NavLink>
</div>
</div>
</nav>
Expand Down
5 changes: 5 additions & 0 deletions src/components/Pages/HomePage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import React from 'react';

export const HomePage: React.FC = () => {
return <h1 className="title">Home Page</h1>;
};
5 changes: 5 additions & 0 deletions src/components/Pages/PageNotFound.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import React from 'react';

export const PageNotFound: React.FC = () => {
return <h1 className="title">Page not found</h1>;
};
170 changes: 153 additions & 17 deletions src/components/PeopleFilters.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,126 @@
export const PeopleFilters = () => {
import React, { useEffect } from 'react';
import { Person } from '../types';
import { useSearchParams } from 'react-router-dom';
import cn from 'classnames';

type Props = {
peoples: Person[];
onFilter: (persons: Person[]) => void;
};

export const PeopleFilters: React.FC<Props> = ({ peoples, onFilter }) => {
const [serchParams, setSerchParams] = useSearchParams();
const query = serchParams.get('query') || '';
const centuries = serchParams.getAll('centuries') || [];
const sex = serchParams.get('sex') || '';

function hangeChangeQuery(event: React.ChangeEvent<HTMLInputElement>) {
const params = new URLSearchParams(serchParams);
const trueQuery = event.target.value.trim();

if (!trueQuery) {
params.delete('query');
} else {
params.set('query', event.target.value.trim());
}

setSerchParams(params);
Comment on lines +17 to +27

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here you always set the query param, even when the input is empty; the requirements (checklist #8/#18) say that when the input is empty there should be no query in the search params. Consider deleting query from params if event.target.value.trim() is an empty string instead of setting it.

}

function hangeChangeSex(sexe: string) {
const params = new URLSearchParams(serchParams);

params.set('sex', sexe);
setSerchParams(params);
}

function hangeChangeAges(n: string) {
const params = new URLSearchParams(serchParams);

if (n === 'All') {
params.delete('centuries');

return setSerchParams(params);
}

const newCenturies = centuries.includes(n)
? centuries.filter(centurie => centurie !== n)
: [...centuries, n];

params.delete('centuries');
newCenturies.forEach(ages => params.append('centuries', ages));

return setSerchParams(params);
}

const handleClearAll = () => {
const params = new URLSearchParams(serchParams);

params.delete('centuries');
params.delete('query');
params.delete('sex');
setSerchParams(params);

return;
};

useEffect(() => {
let filtered = [...peoples];

if (centuries.length > 0) {
filtered = filtered.filter(person => {
Comment on lines +39 to +71

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

sortPeople both updates local masPeople and calls onFilter(sorted), which then changes visiblePeoples in the parent. Since sorted is already stored locally in masPeople and the sort criteria are in the URL, propagating the sorted list upwards is redundant and can make state harder to reason about. Consider relying on URL params plus local state here, or centralizing the sort in the parent, but avoid two independent sorted states.

const century = Math.ceil(person.born / 100);

return centuries.includes(String(century));
});
}

if (sex) {
filtered = filtered.filter(person => person.sex === sex);
}

if (query.trim()) {
filtered = filtered.filter(
person =>
person.name
.toLowerCase()
.trim()
.includes(query.trim().toLowerCase()) ||
person.fatherName
?.toLowerCase()
.trim()
.includes(query.trim().toLowerCase()) ||
person.motherName
?.trim()
.toLowerCase()
.includes(query.trim().toLowerCase()),
);
}

onFilter(filtered);
}, [peoples, serchParams, sex, query, onFilter]);

Check warning on line 101 in src/components/PeopleFilters.tsx

View workflow job for this annotation

GitHub Actions / run_linter (20.x)

React Hook useEffect has a missing dependency: 'centuries'. Either include it or remove the dependency array
Comment on lines +67 to +101

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 effect recalculates filtered based on peoples and search params and then calls onFilter(filtered), while PeopleTable also calls onFilter when sorting. That double responsibility makes the visible list depend on both URL params and internal state, which conflicts with the requirement that filters/sort be controlled by URL search params. Consider centralizing filtering/sorting logic so that it derives solely from peoples + search params in one place.


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={cn(sex === '' ? 'is-active' : '')}
onClick={() => hangeChangeSex('')}
>
All
</a>
<a className="" href="#/people?sex=m">
<a
className={cn(sex === 'm' ? 'is-active' : '')}
onClick={() => hangeChangeSex('m')}
>
Male
</a>
<a className="" href="#/people?sex=f">
<a
className={cn(sex === 'f' ? 'is-active' : '')}
onClick={() => hangeChangeSex('f')}
>
Female
</a>
</p>
Expand All @@ -22,6 +132,8 @@
type="search"
className="input"
placeholder="Search"
value={query}
onChange={event => hangeChangeQuery(event)}
/>

<span className="icon is-left">
Expand All @@ -35,40 +147,55 @@
<div className="level-left">
<a
data-cy="century"
className="button mr-1"
href="#/people?centuries=16"
className={cn(
'button mr-1',
centuries.includes('16') ? 'is-info' : '',
)}
onClick={() => hangeChangeAges('16')}
>
16
</a>

<a
data-cy="century"
className="button mr-1 is-info"
href="#/people?centuries=17"
className={cn(
'button mr-1',
centuries.includes('17') ? 'is-info' : '',
)}
onClick={() => hangeChangeAges('17')}
>
17
</a>

<a
data-cy="century"
className="button mr-1 is-info"
href="#/people?centuries=18"
className={cn(
'button mr-1',
centuries.includes('18') ? 'is-info' : '',
)}
onClick={() => hangeChangeAges('18')}
>
18
</a>

<a
data-cy="century"
className="button mr-1 is-info"
href="#/people?centuries=19"
className={cn(
'button mr-1',
centuries.includes('19') ? 'is-info' : '',
)}
onClick={() => hangeChangeAges('19')}
>
19
</a>

<a
data-cy="century"
className="button mr-1"
href="#/people?centuries=20"
className={cn(
'button mr-1',
centuries.includes('20') ? 'is-info' : '',
)}
onClick={() => hangeChangeAges('20')}
>
20
</a>
Expand All @@ -77,8 +204,11 @@
<div className="level-right ml-4">
<a
data-cy="centuryALL"
className="button is-success is-outlined"
href="#/people"
className={cn(
'button is-success',
centuries.length !== 0 ? 'is-outlined' : '',
)}
onClick={() => hangeChangeAges('All')}
>
All
</a>
Expand All @@ -87,7 +217,13 @@
</div>

<div className="panel-block">
<a className="button is-link is-outlined is-fullwidth" href="#/people">
<a
className={cn(
'button is-link is-fullwidth',
serchParams.size !== 0 ? 'is-outlined' : '',
)}
onClick={handleClearAll}
>
Reset all filters
</a>
</div>
Expand Down
Loading
Loading