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
71 changes: 13 additions & 58 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,73 +1,28 @@
import React from 'react';
import './App.scss';
import { peopleFromServer } from './data/people';
import { UserSelect } from './components/UserSelect';
import { Person } from './types/Person';

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

const handleSelect = (person: Person | null) => {
setSelectedPerson(person);
};

return (
<div className="container">
<main className="section is-flex is-flex-direction-column">
<h1 className="title" data-cy="title">
{`${name} (${born} - ${died})`}
{selectedPerson
? `${selectedPerson.name} (${selectedPerson.born} - ${selectedPerson.died})`
: 'No selected person'}
</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>
<UserSelect people={peopleFromServer} onSelected={handleSelect} />
</main>
</div>
);
Expand Down
100 changes: 100 additions & 0 deletions src/components/UserSelect.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { Person } from '../types/Person';
import React, { useCallback, useMemo } from 'react';
import debounce from 'lodash.debounce';
import classNames from 'classnames';

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

export const UserSelect = ({
people,
delay = 300,
onSelected = () => {},
}: Props) => {
const [query, setQuery] = React.useState('');
const [appliedQuery, setAppliedQuery] = React.useState('');
const [isDropdownOpen, setIsDropdownOpen] = React.useState(false);

const applyQuery = useCallback(debounce(setAppliedQuery, delay), [delay]);

Check warning on line 21 in src/components/UserSelect.tsx

View workflow job for this annotation

GitHub Actions / run_linter (20.x)

React Hook useCallback received a function whose dependencies are unknown. Pass an inline function instead

const handleQueryChange = (event: React.ChangeEvent<HTMLInputElement>) => {
setQuery(event.target.value);
applyQuery(event.target.value);
onSelected(null);
};

const filterPeople = useMemo(() => {
if (!appliedQuery.trim()) {
return people;
}

return people.filter(person => person.name.includes(appliedQuery));
}, [people, appliedQuery]);

return (
<>
<div className={classNames('dropdown', { 'is-active': isDropdownOpen })}>
<div className="dropdown-trigger">
<input
type="text"
placeholder="Enter a part of the name"
className="input"
data-cy="search-input"
value={query}
onChange={handleQueryChange}
onFocus={() => setIsDropdownOpen(true)}
onBlur={() => setIsDropdownOpen(false)}
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 onBlur handler here causes a problem: when you click on a suggestion, this onBlur event fires before the suggestion's onClick event on line 61. This closes the dropdown, preventing the onClick from ever firing, which means suggestions can't be selected.

As mentioned in the previous review, replacing onClick with onMouseDown on the suggestion items will fix this because mousedown fires before blur.

/>
Comment on lines +41 to +50
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For a more intuitive user experience, consider adding an onBlur event handler to this input. This would allow you to close the dropdown (setIsDropdownOpen(false)) when the user clicks away from the component.

</div>

{isDropdownOpen && (
<div className="dropdown-menu" role="menu" data-cy="suggestions-list">
<div className="dropdown-content">
{filterPeople.map(person => (
<div
className="dropdown-item"
key={person.slug}
data-cy="suggestion-item"
onClick={() => {
onSelected(person);
setQuery(person.name);
setAppliedQuery(person.name);
setIsDropdownOpen(false);
}}
Comment on lines +61 to +66
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 requirement to 'close the list' after selecting an item is not met. You should update the state to close the dropdown here.

Additionally, appliedQuery is not updated after selection. This can lead to showing a stale list of suggestions if the user reopens the dropdown. It would be better to also update appliedQuery to match the selected person's name.

Comment on lines +61 to +66
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 dropdown list should close after a user selects an item, as required by the task. You can achieve this by updating the state that controls the dropdown's visibility inside this onClick handler.

>
<p
className={classNames({
'has-text-link': person.sex === 'm',
'has-text-danger': person.sex === 'f',
})}
>
{person.name}
</p>
</div>
))}
</div>
</div>
)}
</div>

{filterPeople.length === 0 && appliedQuery.trim().length > 0 && (
<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>
)}
</>
);
};
Loading