Skip to content

solution #1839

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.

Sign up for GitHub

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

Open
Alternate001 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

solution #1839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f19a253
solution
Alternate001 Jul 7, 2026
c087603
solution
Alternate001 Jul 7, 2026
11871d1
solution
Alternate001 Jul 7, 2026
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ implement the ability to filter and sort people in the table.
- Implement a solution following the [React task guideline](https://github.com/mate-academy/react_task-guideline#react-tasks-guideline).
- Use the [React TypeScript cheat sheet](https://mate-academy.github.io/fe-program/js/extra/react-typescript).
- Open one more terminal and run tests with `npm test` to ensure your solution is correct.
- Replace `<your_account>` with your Github username in the [DEMO LINK](https://<your_account>.github.io/react_people-table-advanced/) and add it to the PR description.
- Replace `<your_account>` with your Github username in the [DEMO LINK](https://Alternate001.github.io/react_people-table-advanced/) and add it to the PR description.
75 changes: 67 additions & 8 deletions src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,79 @@
import './App.scss';
import {
Link,
Navigate,
Route,
Routes,
useLocation,
useSearchParams,
} from 'react-router-dom';
import { PeoplePage } from './components/PeoplePage';
import { Navbar } from './components/Navbar';

import './App.scss';
const People = () => {
return (
<>
<PeoplePage />
</>
);
};

export const App = () => {
const location = useLocation();
const [searchParams] = useSearchParams();

return (
<div data-cy="app">
<Navbar />
<nav
data-cy="nav"
className="navbar is-fixed-top has-shadow"
role="navigation"
aria-label="main navigation"
>
<div className="container">
<div className="navbar-brand">
<Link
className={
location.pathname === '/'
? 'navbar-item has-background-grey-lighter'
: 'navbar-item'
}
to={'/'}
>
Home
</Link>
<Link
className={
location.pathname.startsWith('/people')
? 'navbar-item has-background-grey-lighter'
: 'navbar-item'
}
to={{
pathname: `/people`,
search: searchParams.toString()
? `?${searchParams.toString()}`
: '',
}}

@mateacademy-ai-mentor mateacademy-ai-mentor Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your use of useLocation and useSearchParams to build the People link to object preserves all current search params when navigating back to /people, satisfying the requirement to keep filters/sorting in the URL when using the People nav link.

>
People

@mateacademy-ai-mentor mateacademy-ai-mentor Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Binding the input value to searchParams.get('query') ?? '' ensures the NameFilter is controlled by the URL (e.g. persisted on reload), which matches the requirement and fixes the previous review issue.

</Link>
</div>
</div>
</nav>

<div className="section">
<main className="section">
Comment on lines +58 to +63

@mateacademy-ai-mentor mateacademy-ai-mentor Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

NameFilter is required to update the query search param and remove it when the input is empty, but this input currently has no value or onChange handler, so no URL search param is ever set or cleared. Consider using useSearchParams here to set query on change and pass null when the input becomes empty.

<div className="container">
<h1 className="title">Home Page</h1>
<h1 className="title">Page not found</h1>
<PeoplePage />
<Routes>
<Route path="/" element={<h1 className="title">Home Page</h1>} />
<Route path="home" element={<Navigate to="/" replace />} />
<Route path="people" element={<People />} />
<Route path="/people/:slug" element={<People />} />
<Route
path="*"
element={<h1 className="title">Page not found</h1>}
/>
</Routes>
</div>
</div>
</main>
</div>
);
};
2 changes: 1 addition & 1 deletion src/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function wait(delay: number) {
return new Promise(resolve => setTimeout(resolve, delay));
}

export async function getPeople(): Promise<Person[]> {
export function getPeople(): Promise<Person[]> {
// keep this delay for testing purpose
return wait(500)
.then(() => fetch(API_URL))
Expand Down
131 changes: 99 additions & 32 deletions src/components/PeopleFilters.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,67 @@
import { SearchLink } from './SearchLink';
import { useSearchParams } from 'react-router-dom';

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

const getCenturyParams = (century: string) => {
const newCenturies = selectedCenturies.includes(century)
? selectedCenturies.filter(item => item !== century)
: [...selectedCenturies, century];

return {
centuries: newCenturies.length > 0 ? newCenturies : null,
};
};

const InputHandler = (value: string) => {
const copy = new URLSearchParams(searchParams);

copy.set('query', value);
if (value === '') {
copy.delete('query');
}

setSearchParams(copy);
};

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

<p className="panel-tabs" data-cy="SexFilter">
<a className="is-active" href="#/people">
<SearchLink
className={searchParams.get('sex') === null ? 'is-active' : ''}
params={{ sex: null }}
>
All
</a>
<a className="" href="#/people?sex=m">
</SearchLink>
<SearchLink
className={searchParams.get('sex') === 'm' ? 'is-active' : ''}
params={{ sex: 'm' }}
Comment on lines +36 to +42

@mateacademy-ai-mentor mateacademy-ai-mentor Jul 7, 2026

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 People nav link always goes to /people and drops any existing search params, but the spec requires that filters/sorting be preserved when clicking People. Consider using useSearchParams or useLocation().search here and pass a to object (or SearchLink) that includes the current search string so the existing params are kept.

>
Male
</a>
<a className="" href="#/people?sex=f">
</SearchLink>
<SearchLink
className={searchParams.get('sex') === 'f' ? 'is-active' : ''}
params={{ sex: 'f' }}
>
Female
</a>
</SearchLink>
</p>

<div className="panel-block">
<p className="control has-icons-left">
<input
value={searchParams.get('query') ?? ''}
data-cy="NameFilter"
type="search"
className="input"
placeholder="Search"
onChange={e => {
InputHandler(e.target.value);
}}
/>

<span className="icon is-left">
Expand All @@ -33,63 +73,90 @@ export const PeopleFilters = () => {
<div className="panel-block">
<div className="level is-flex-grow-1 is-mobile" data-cy="CenturyFilter">
<div className="level-left">
<a
<SearchLink
data-cy="century"
className="button mr-1"
href="#/people?centuries=16"
className={
searchParams.getAll('centuries').includes('16')
? 'is-info button mr-1'
: 'button mr-1'
}
params={getCenturyParams('16')}
>
16
</a>
</SearchLink>

<a
<SearchLink
data-cy="century"
className="button mr-1 is-info"
href="#/people?centuries=17"
className={
searchParams.getAll('centuries').includes('17')
? 'is-info button mr-1'
: 'button mr-1'
}
params={getCenturyParams('17')}
>
17
</a>
</SearchLink>

<a
<SearchLink
data-cy="century"
className="button mr-1 is-info"
href="#/people?centuries=18"
className={
searchParams.getAll('centuries').includes('18')
? 'is-info button mr-1'
: 'button mr-1'
}
params={getCenturyParams('18')}
>
Comment on lines 96 to 108

@mateacademy-ai-mentor mateacademy-ai-mentor Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In inputFilter, item will be '' (empty string) when the URL has ?query=, and currently you only check for item === undefined. Because every string includes '', this makes all people match, whereas the requirement says empty input / empty query should mean no filter. Consider treating both undefined and '' as no query (e.g., return true early if !item).

18
</a>
</SearchLink>

<a
<SearchLink
data-cy="century"
className="button mr-1 is-info"
href="#/people?centuries=19"
className={
searchParams.getAll('centuries').includes('19')
? 'is-info button mr-1'
: 'button mr-1'
}
params={getCenturyParams('19')}
>
19
</a>
</SearchLink>

<a
<SearchLink
data-cy="century"
className="button mr-1"
href="#/people?centuries=20"
className={
searchParams.getAll('centuries').includes('20')
? 'is-info button mr-1'
: 'button mr-1'
}
params={getCenturyParams('20')}
>
20
</a>
</SearchLink>
</div>

<div className="level-right ml-4">
<a
<SearchLink
data-cy="centuryALL"
className="button is-success is-outlined"
href="#/people"
className={
searchParams.getAll('centuries').length === 0
? 'is-success button'
: 'is-success is-outlined button'
}
params={{ centuries: null }}
>
All
</a>
</SearchLink>
</div>
</div>
</div>

<div className="panel-block">
<a className="button is-link is-outlined is-fullwidth" href="#/people">
<SearchLink
className="button is-link is-outlined is-fullwidth"
params={{ centuries: null, sex: null, query: null }}
>
Reset all filters
</a>
</SearchLink>
</div>
</nav>
);
Expand Down
54 changes: 36 additions & 18 deletions src/components/PeoplePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,50 @@
import { PeopleFilters } from './PeopleFilters';
import { Loader } from './Loader';
import { PeopleTable } from './PeopleTable';
import { useState } from 'react';
import { getPeople } from '../api';
import { useEffect } from 'react';
import { Person } from '../types';

export const PeoplePage = () => {
const [isLoading, setIsLoading] = useState(true);
const [hasError, setHasError] = useState(false);
const [list, setList] = useState<Person[]>([]);

useEffect(() => {
getPeople()
.then(people => {
setList(people);
setIsLoading(false);
})
.catch(() => {
setHasError(true);
setIsLoading(false);
});
}, []);

return (
<>
<h1 className="title">People Page</h1>

<div className="block">
<div className="columns is-desktop is-flex-direction-row-reverse">
<div className="column is-7-tablet is-narrow-desktop">
<PeopleFilters />
</div>

<div className="column">
<div className="box table-container">
<Loader />

<p data-cy="peopleLoadingError">Something went wrong</p>

<p data-cy="noPeopleMessage">There are no people on the server</p>

<p>There are no people matching the current search criteria</p>

<PeopleTable />
{isLoading ? (
<Loader />
) : hasError ? (
<p data-cy="peopleLoadingError" className="has-text-danger">
Something went wrong
</p>
) : list.length === 0 ? (
<p data-cy="noPeopleMessage">There are no people on the server</p>
) : (
<div className="columns">
<div className="column">
<PeopleTable list={list} />
</div>

@mateacademy-ai-mentor mateacademy-ai-mentor Jul 7, 2026

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 People navigation link currently always goes to /people and drops any active search params (filters/sorting). The requirements state that search params must be kept when navigating within the People page, including when clicking this link. Consider building the to using current search params (e.g. via useSearchParams or SearchLink) so filters and sort state are preserved.

<div className="column is-7-tablet is-narrow-desktop">
<PeopleFilters />
</div>
</div>
</div>
)}
</div>
</>
);
Comment on lines 45 to 50

@mateacademy-ai-mentor mateacademy-ai-mentor Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Currently the NameFilter input is uncontrolled and does not update the query search param, which is required. You need to capture changes (e.g., with onChange) and update search params so that non‑empty text sets query and empty text removes query.

Expand Down
Loading
Loading