-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
solution #1175
base: master
Are you sure you want to change the base?
solution #1175
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import { | ||
Navigate, | ||
Route, | ||
HashRouter as Router, | ||
Routes, | ||
} from 'react-router-dom'; | ||
import { App } from './App'; | ||
import { HomePage } from './components/HomePage/HomePage'; | ||
import { NotFoundPage } from './components/NotFoundPage/NotFoundPage'; | ||
import { PeoplePage } from './components/PeoplePage'; | ||
|
||
export const Root = () => { | ||
return ( | ||
<Router> | ||
<Routes> | ||
<Route path="/" element={<App />}> | ||
<Route index element={<HomePage />} /> | ||
<Route path="home" element={<Navigate to=".." replace={true} />} /> | ||
<Route path="people" element={<PeoplePage />}> | ||
<Route path=":slug?" element={<PeoplePage />} /> | ||
</Route> | ||
<Route path="*" element={<NotFoundPage />} /> | ||
</Route> | ||
</Routes> | ||
</Router> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
export const ErrorMessage = () => { | ||
return ( | ||
<p data-cy="peopleLoadingError" className="has-text-danger"> | ||
Something went wrong | ||
</p> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export const HomePage = () => { | ||
return <h1 className="title">Home Page</h1>; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export const NotFoundPage = () => { | ||
return <h1 className="title">Page not found</h1>; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,45 @@ | ||
import { useSearchParams } from 'react-router-dom'; | ||
import { getSearchWith, SearchParams } from '../utils/searchHelper'; | ||
import { SearchLink } from './SearchLink'; | ||
import { Sex } from '../types/Sex'; | ||
import classNames from 'classnames'; | ||
import { CENTURIES } from '../constants/constants'; | ||
|
||
export const PeopleFilters = () => { | ||
const [searchParams, setSearchParams] = useSearchParams(); | ||
const query = searchParams.get('query') || ''; | ||
const sex = searchParams.get('sex'); | ||
const centuries = searchParams.getAll('centuries'); | ||
|
||
function setSearchWith(params: SearchParams) { | ||
const search = getSearchWith(searchParams, params); | ||
|
||
setSearchParams(search); | ||
} | ||
|
||
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={classNames({ 'is-active': sex === null })} | ||
params={{ sex: null }} | ||
> | ||
All | ||
</a> | ||
<a className="" href="#/people?sex=m"> | ||
</SearchLink> | ||
<SearchLink | ||
className={classNames({ 'is-active ': sex === Sex.Male })} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is an extra space in the class name 'is-active ' which might cause styling issues. Consider removing the trailing space to ensure consistent styling. |
||
params={{ sex: Sex.Male }} | ||
> | ||
Male | ||
</a> | ||
<a className="" href="#/people?sex=f"> | ||
</SearchLink> | ||
<SearchLink | ||
className={classNames({ 'is-active': sex === Sex.Female })} | ||
params={{ sex: Sex.Female }} | ||
> | ||
Female | ||
</a> | ||
</SearchLink> | ||
</p> | ||
|
||
<div className="panel-block"> | ||
|
@@ -22,74 +49,66 @@ export const PeopleFilters = () => { | |
type="search" | ||
className="input" | ||
placeholder="Search" | ||
value={query} | ||
onChange={event => | ||
setSearchWith({ query: event.target.value || null }) | ||
} | ||
/> | ||
|
||
<span className="icon is-left"> | ||
<i className="fas fa-search" aria-hidden="true" /> | ||
<i | ||
className="fas fa-search" | ||
aria-hidden="true" | ||
onClick={() => setSearchWith({ query })} | ||
/> | ||
</span> | ||
</p> | ||
</div> | ||
|
||
<div className="panel-block"> | ||
<div className="level is-flex-grow-1 is-mobile" data-cy="CenturyFilter"> | ||
<div className="level-left"> | ||
<a | ||
data-cy="century" | ||
className="button mr-1" | ||
href="#/people?centuries=16" | ||
> | ||
16 | ||
</a> | ||
|
||
<a | ||
data-cy="century" | ||
className="button mr-1 is-info" | ||
href="#/people?centuries=17" | ||
> | ||
17 | ||
</a> | ||
|
||
<a | ||
data-cy="century" | ||
className="button mr-1 is-info" | ||
href="#/people?centuries=18" | ||
> | ||
18 | ||
</a> | ||
|
||
<a | ||
data-cy="century" | ||
className="button mr-1 is-info" | ||
href="#/people?centuries=19" | ||
> | ||
19 | ||
</a> | ||
|
||
<a | ||
data-cy="century" | ||
className="button mr-1" | ||
href="#/people?centuries=20" | ||
> | ||
20 | ||
</a> | ||
{CENTURIES.map(century => ( | ||
<> | ||
<SearchLink | ||
data-cy="century" | ||
key={century} | ||
params={{ | ||
centuries: centuries.includes(century) | ||
? centuries.filter(selCentury => selCentury !== century) | ||
: [...centuries, century], | ||
}} | ||
className={classNames('button mr-1', { | ||
'is-info': centuries.includes(century), | ||
})} | ||
> | ||
{century} | ||
</SearchLink> | ||
</> | ||
))} | ||
</div> | ||
|
||
<div className="level-right ml-4"> | ||
<a | ||
<SearchLink | ||
data-cy="centuryALL" | ||
className="button is-success is-outlined" | ||
href="#/people" | ||
className={classNames('button is-success', { | ||
'is-outlined': !!centuries.length, | ||
})} | ||
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={{ query: null, centuries: null, sex: null }} | ||
> | ||
Reset all filters | ||
</a> | ||
</SearchLink> | ||
</div> | ||
</nav> | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,29 +1,99 @@ | ||
import { PeopleFilters } from './PeopleFilters'; | ||
import { Loader } from './Loader'; | ||
import { PeopleTable } from './PeopleTable'; | ||
import { ErrorMessage } from './ErrorMessage/ErrorMessage'; | ||
import { useEffect, useState } from 'react'; | ||
import { Person } from '../types'; | ||
import { getPeople } from '../api'; | ||
import { useSearchParams } from 'react-router-dom'; | ||
import { Parameter } from '../types/Parameter'; | ||
import { Order } from '../types/Order'; | ||
|
||
export const PeoplePage = () => { | ||
const [searchParams] = useSearchParams(); | ||
const query = searchParams.get('query') || ''; | ||
const sex = searchParams.get('sex'); | ||
const centuries = searchParams.getAll('centuries'); | ||
|
||
const order = searchParams.get('order'); | ||
const sort = searchParams.get('sort'); | ||
|
||
const [people, setPeople] = useState<Person[]>([]); | ||
const [isLoading, setIsLoading] = useState(false); | ||
const [isError, setIsError] = useState(false); | ||
|
||
useEffect(() => { | ||
setIsLoading(true); | ||
|
||
getPeople() | ||
.then(setPeople) | ||
.catch(() => setIsError(true)) | ||
.finally(() => setIsLoading(false)); | ||
}, []); | ||
|
||
const filteredPeople = people | ||
.filter( | ||
person => | ||
person.name.toLocaleLowerCase().includes(query.toLowerCase()) || | ||
person.fatherName?.toLocaleLowerCase().includes(query.toLowerCase()) || | ||
person.motherName?.toLowerCase().includes(query.toLowerCase()), | ||
Comment on lines
+37
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using |
||
) | ||
.filter(person => { | ||
if (!sex) { | ||
return true; | ||
} | ||
|
||
return sex === person.sex; | ||
}) | ||
.filter(person => { | ||
if (!centuries.length) { | ||
return true; | ||
} | ||
|
||
return centuries.includes(Math.ceil(person.born / 100).toString()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure that the |
||
}) | ||
.sort((person1: Person, person2: Person): number => { | ||
let coefficient = 0; | ||
|
||
if (sort === Parameter.Name.toLowerCase()) { | ||
coefficient = person1.name.localeCompare(person2.name); | ||
} else if (sort === Parameter.Sex.toLowerCase()) { | ||
coefficient = person1.sex.localeCompare(person2.sex); | ||
} else if (sort === Parameter.Born.toLowerCase()) { | ||
coefficient = person1.born - person2.born; | ||
} else if (sort === Parameter.Died.toLowerCase()) { | ||
coefficient = person1.died - person2.died; | ||
} | ||
|
||
return order === Order.desc ? -coefficient : coefficient; | ||
}); | ||
|
||
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> | ||
{!isLoading && !isError && ( | ||
<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> | ||
{isLoading && <Loader />} | ||
{isError && <ErrorMessage />} | ||
|
||
<PeopleTable /> | ||
{!isLoading && | ||
!isError && | ||
(people.length > 0 ? ( | ||
<PeopleTable people={filteredPeople} /> | ||
) : ( | ||
<p data-cy="noPeopleMessage"> | ||
There are no people on the server | ||
</p> | ||
))} | ||
</div> | ||
</div> | ||
</div> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nested route for
PeoplePage
seems to renderPeoplePage
twice: once for thepeople
path and again for the optional:slug?
parameter. This might lead to rendering issues or unexpected behavior. Consider reviewing the intended behavior and adjusting the route structure if necessary.