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

add solution #760

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
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
72 changes: 13 additions & 59 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,73 +1,27 @@
import React from 'react';
import React, { useState } from 'react';
import './App.scss';
import { peopleFromServer } from './data/people';
import { Autocomplete } from './components/Autocomplete';
import { Person } from './types/Person';

export const App: React.FC = () => {
const { name, born, died } = peopleFromServer[0];
const [selectedPerson, setSelectedPerson] = useState<Person | null>(null);

const getTitle = selectedPerson
? `${selectedPerson.name} (${selectedPerson.born} - ${selectedPerson.died})`
: 'No selected person';

return (
<div className="container">
<main className="section is-flex is-flex-direction-column">
<h1 className="title" data-cy="title">
{`${name} (${born} - ${died})`}
{getTitle}
</h1>

<div className="dropdown is-active">
<div className="dropdown-trigger">
<input
type="text"
placeholder="Enter a part of the name"
className="input"
data-cy="search-input"
/>
</div>

<div className="dropdown-menu" role="menu" data-cy="suggestions-list">
<div className="dropdown-content">
<div className="dropdown-item" data-cy="suggestion-item">
<p className="has-text-link">Pieter Haverbeke</p>
</div>

<div className="dropdown-item" data-cy="suggestion-item">
<p className="has-text-link">Pieter Bernard Haverbeke</p>
</div>

<div className="dropdown-item" data-cy="suggestion-item">
<p className="has-text-link">Pieter Antone Haverbeke</p>
</div>

<div className="dropdown-item" data-cy="suggestion-item">
<p className="has-text-danger">Elisabeth Haverbeke</p>
</div>

<div className="dropdown-item" data-cy="suggestion-item">
<p className="has-text-link">Pieter de Decker</p>
</div>

<div className="dropdown-item" data-cy="suggestion-item">
<p className="has-text-danger">Petronella de Decker</p>
</div>

<div className="dropdown-item" data-cy="suggestion-item">
<p className="has-text-danger">Elisabeth Hercke</p>
</div>
</div>
</div>
</div>

<div
className="
notification
is-danger
is-light
mt-3
is-align-self-flex-start
"
role="alert"
data-cy="no-suggestions-message"
>
<p className="has-text-danger">No matching suggestions</p>
</div>
<Autocomplete
onSelected={setSelectedPerson}
people={peopleFromServer}
/>
</main>
</div>
);
Expand Down
4 changes: 4 additions & 0 deletions src/components/Autocomplete.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.dropdown-item:hover {
cursor: pointer;
background-color: #e5e5e5;
}
107 changes: 107 additions & 0 deletions src/components/Autocomplete.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import classNames from 'classnames';
import { Person } from '../types/Person';
import { useMemo, useState } from 'react';
import debounce from 'lodash.debounce';
import './Autocomplete.scss';

type Props = {
people: Person[];
onSelected: (person: Person | null) => void;
delay?: number;
};

export const Autocomplete: React.FC<Props> = ({
people,
onSelected = () => {},
delay = 300,
}) => {
const [focusOnInput, setFocusOnInput] = useState(false);
const [appliedQuery, setAppliedQuery] = useState('');
const [query, setQuery] = useState('');

const applyQuery = debounce(setAppliedQuery, delay);

const handleInputChange = (event: React.ChangeEvent<HTMLInputElement>) => {

Choose a reason for hiding this comment

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

The naming convention for this method could be improved. According to the conventions, it's better to name the function onInputChange instead of handleInputChange.

const value = event.target.value;

setQuery(value);
applyQuery(value);

Choose a reason for hiding this comment

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

You are not checking if the user entered spaces only. This can lead to unnecessary function calls. You should add a condition to check if the value is not empty or doesn't contain spaces only before calling applyQuery(value).

setFocusOnInput(true);
onSelected(null);
};

const handleSelectPerson = (person: Person) => {

Choose a reason for hiding this comment

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

The naming convention for this method could be improved. According to the conventions, it's better to name the function onSelectPerson instead of handleSelectPerson.

onSelected(person);
setFocusOnInput(false);
setQuery(person.name);
};

const filteredPeople = useMemo(() => {
return people.filter(person =>
person.name

Choose a reason for hiding this comment

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

You are directly interacting with DOM here. It's recommended to use React as much as possible. You can use the trim() function when setting the state of appliedQuery instead of using it here.

.toLowerCase()
.trim()
.includes(appliedQuery.toLowerCase().trim()),
);
}, [appliedQuery, people]);

return (
<div
className={classNames('dropdown', {
'is-active': focusOnInput,
})}
>
<div className="dropdown-trigger">
<input
type="text"
placeholder="Enter a part of the name"
className="input"
value={query}
data-cy="search-input"
onFocus={() => setFocusOnInput(true)}
onBlur={() => setFocusOnInput(false)}
onChange={handleInputChange}
/>
</div>

{focusOnInput && (
<div className="dropdown-menu" role="menu" data-cy="suggestions-list">
<div className="dropdown-content">
{filteredPeople.length > 0 ? (
filteredPeople.map(person => (
<div
key={person.slug}

Choose a reason for hiding this comment

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

You are generating a key on render. This can lead to performance issues. It's recommended to generate keys during the creation of items. Here is a link explaining why.

className="dropdown-item"
data-cy="suggestion-item"
onMouseDown={() => handleSelectPerson(person)}
>
<p
className={classNames('has-text-link', {
'has-text-danger': person.sex === 'f',
})}
>
{person.name}
</p>
</div>
))
) : (
<div
className="
notification
is-danger
is-light
mt-3
is-align-self-flex-start
"
role="alert"
data-cy="no-suggestions-message"
>
<p className="has-text-danger">No matching suggestions</p>
</div>
)}
</div>
</div>
)}
</div>
);
};
4 changes: 4 additions & 0 deletions src/css/Autocomplete.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.dropdown-item:hover {
cursor: pointer;
background-color: #e5e5e5;
}
Loading