Skip to content
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

implement the ability to filter and sort people in the table #516

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
10 changes: 4 additions & 6 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
import { PeoplePage } from './components/PeoplePage';
import { Navbar } from './components/Navbar';

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

import { Navbar } from './components/Navbar/Navbar';

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

<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
25 changes: 25 additions & 0 deletions src/Root.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import {
Navigate,
Route,
HashRouter as Router,
Routes,
} from 'react-router-dom';

import { App } from './App';
import { HomePage } from './pages/HomePage';
import { PeoplePage } from './pages/PeoplePage';
import { NotFoundPage } from './pages/NotFoundPage';

export const Root = () => (
<Router>
<Routes>
<Route path="/" element={<App />}>
<Route index element={<HomePage />} />
<Route path="home" element={<Navigate to="/" replace />} />
<Route path="people" element={<PeoplePage />} />
<Route path="people/:slug?" element={<PeoplePage />} />

Choose a reason for hiding this comment

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

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

<Route path="*" element={<NotFoundPage />} />
</Route>
</Routes>
</Router>
);
110 changes: 110 additions & 0 deletions src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,29 @@ import { Person } from './types/Person';
// eslint-disable-next-line max-len
const API_URL = 'https://mate-academy.github.io/react_people-table/api/people.json';

export const NOT_SET_VALUE = '-';

export enum ColumnNames {
Name = 'Name',
Sex = 'Sex',
Born = 'Born',
Died = 'Died',
}

export enum PersonSex {
All = '',
Male = 'm',
Female = 'f',
}

Choose a reason for hiding this comment

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

Move constants to the constants file/folder and enums/types to the types folder


export type FilterType = {
query: string | '';
centuries: string[];
sex: string | '';
};

Choose a reason for hiding this comment

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

Suggested change
export type FilterType = {
query: string | '';
centuries: string[];
sex: string | '';
};
export type FilterType = {
query: string;
centuries: string[];
sex: string;
};

An empty string is still a string


export const CENTURIES = ['16', '17', '18', '19', '20'];

function wait(delay: number) {
return new Promise(resolve => setTimeout(resolve, delay));
}
Expand All @@ -13,3 +36,90 @@ export async function getPeople(): Promise<Person[]> {
.then(() => fetch(API_URL))
.then(response => response.json());
}

function getParent(people: Person[], parentName: string) {
return people.find(({ name }) => name === parentName);
}

export function addParent(people: Person[]) {
return people.map(person => {

Choose a reason for hiding this comment

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

Move all helpers to the helpers or utils folder.

let mother;
let father;

if (person.motherName) {
mother = getParent(people, person.motherName);
}

if (person.fatherName) {
father = getParent(people, person.fatherName);
}

return {
...person,
mother,
father,
};

Choose a reason for hiding this comment

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

Suggested change
let mother;
let father;
if (person.motherName) {
mother = getParent(people, person.motherName);
}
if (person.fatherName) {
father = getParent(people, person.fatherName);
}
return {
...person,
mother,
father,
};
const { motherName, fatherName } = person;
return {
...person,
mother: motherName ? getParent(people, motherName) : motherName,
father: getParent(people, fatherName), // or like this, and if no fatherName just return from the getParent function
};

});
}

export const sortPeople = (people: Person[], sortParam: string) => {
if (sortParam) {
return [...people].sort((a, b) => {
switch (sortParam) {
case ('name'): {

Choose a reason for hiding this comment

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

sortParam should be enum

return a[sortParam].localeCompare(b[sortParam]);
}

case ('sex'): {
return a[sortParam].localeCompare(b[sortParam]);
}

case ('born'): {
return a[sortParam] - (b[sortParam]);
}

case ('died'): {
return a[sortParam] - (b[sortParam]);
}

Choose a reason for hiding this comment

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

Suggested change
case ('born'): {
return a[sortParam] - (b[sortParam]);
}
case ('died'): {
return a[sortParam] - (b[sortParam]);
}
case (SortParam.born):
case (SortParam.died): {
return a[sortParam] - (b[sortParam]);
}

You can combine cases if they do a similar job


default: {
return 0;
}
}
});
}

return [...people];
};

export const filterPeople = (
filterOption: FilterType,
people: Person[],
) => {
return people
.filter(person => {
if (filterOption.sex) {
return person.sex === filterOption.sex;
}

return person;
})
.filter(person => {

Choose a reason for hiding this comment

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

You iterate through an array of people every time even some of filterOption is not set

if (filterOption.centuries.length) {
const personBirthCentury = Math.ceil(person.born / 100);

return filterOption.centuries.includes(personBirthCentury.toString());
}

return person;
})
.filter(person => {
if (filterOption.query) {
return person.name.includes(filterOption.query)
|| person.motherName?.includes(filterOption.query)
|| person.fatherName?.includes(filterOption.query);
}

return person;
});
};
51 changes: 51 additions & 0 deletions src/components/ColumnNameItem/ColumnNameItem.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import React from 'react';
import { useSearchParams } from 'react-router-dom';

import { ColumnNames } from '../../api';
import { SearchLink } from '../SearchLink';

type Props = {
value: keyof typeof ColumnNames,
};

export const ColumnName: React.FC<Props> = ({ value }) => {
const [searchParams] = useSearchParams();

const lowerCasedValue = value.toLowerCase();

const sort = searchParams.get('sort') || '';
const order = searchParams.get('order') || '';

const getSortParams = (sortOption: string) => {
if (sortOption !== sort) {
return { sort: sortOption, order: null };
}

if (sort === sortOption && !order) {
return { sort: sortOption, order: 'desc' };
}

return { sort: null, order: null };
};
Comment on lines +20 to +30

Choose a reason for hiding this comment

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

It is better to define this function outside the component so that it is not created every time a new render is performed.


return (
<th key={value}>
<span className="is-flex is-flex-wrap-nowrap">
{value}
<SearchLink
params={getSortParams(lowerCasedValue)}
>
<span className="icon">
{lowerCasedValue !== sort && (<i className="fas fa-sort" />)}
{lowerCasedValue === sort && !order && (
<i className="fas fa-sort-up" />
)}
{order && lowerCasedValue === sort && (

Choose a reason for hiding this comment

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

DRY, create variable for repeatable part of code

<i className="fas fa-sort-down" />
)}
</span>
</SearchLink>
</span>
</th>
);
};
1 change: 1 addition & 0 deletions src/components/ColumnNameItem/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './ColumnNameItem';
34 changes: 34 additions & 0 deletions src/components/FilterCenturyItem/FilterCenturyItem.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import React from 'react';
import { useSearchParams } from 'react-router-dom';
import cn from 'classnames';
import { SearchLink } from '../SearchLink';

type Props = {
value: string,
};

export const CenturyItem: React.FC<Props> = ({ value }) => {
const [searchParams] = useSearchParams();
const centuries = searchParams.getAll('centuries') || [];
const isSelected = centuries.includes(value);

const toggleCentury = (curCentury: string) => {
return centuries.includes(curCentury)
? centuries.filter((age: string) => age !== curCentury)
: [...centuries, curCentury];
};

return (
<SearchLink
data-cy="century"
className={cn(
'button',
'mr-1',
{ 'is-info': isSelected },
)}
params={{ centuries: toggleCentury(value) }}
>
{value}
</SearchLink>
);
};
1 change: 1 addition & 0 deletions src/components/FilterCenturyItem/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './FilterCenturyItem';
33 changes: 33 additions & 0 deletions src/components/FilterSexItem/FilterSexItem.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import React from 'react';
import { useSearchParams } from 'react-router-dom';
import cn from 'classnames';

import { SearchLink } from '../SearchLink';
import { PersonSex } from '../../api';

type Props = {
sexValue: keyof typeof PersonSex,
};

export const PersonSexItem: React.FC<Props> = ({ sexValue }) => {
const [searchParams] = useSearchParams();
const sex = searchParams.get('sex') || '';
let sexParam: string | null = PersonSex[sexValue];
let isActive = sex === sexParam;

if (sexValue === 'All') {
sexParam = null;
isActive = !sex;
}

return (
<SearchLink
className={cn(
{ 'is-active': isActive },
)}
params={{ sex: sexParam }}
>
{sexValue}
</SearchLink>
);
};
1 change: 1 addition & 0 deletions src/components/FilterSexItem/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './FilterSexItem';
24 changes: 0 additions & 24 deletions src/components/Navbar.tsx

This file was deleted.

35 changes: 35 additions & 0 deletions src/components/Navbar/Navbar.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { NavLink } from 'react-router-dom';
import cn from 'classnames';

export const Navbar = () => {
const getLinkClass = ({ isActive }: { isActive: boolean }) => cn(
'navbar-item',
{ 'has-background-grey-lighter': isActive },
);
Comment on lines +6 to +9

Choose a reason for hiding this comment

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

It is better to define this function outside the component so that it is not created every time a new render is performed.


return (
<nav
data-cy="nav"
className="navbar is-fixed-top has-shadow"
role="navigation"
aria-label="main navigation"
>
<div className="container">
<div className="navbar-brand">

<NavLink className={getLinkClass} to="/">
Home
</NavLink>

<NavLink
aria-current="page"
className={getLinkClass}
to="/people"
>
People
</NavLink>
</div>
</div>
</nav>
);
};
1 change: 1 addition & 0 deletions src/components/Navbar/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './Navbar';
Loading