Skip to content
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ that will suggest people matching an entered text.
- Install Prettier Extention and use this [VSCode settings](https://mate-academy.github.io/fe-program/tools/vscode/settings.json) to enable format on save.
- 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).
- Replace `<your_account>` with your Github username in the [DEMO LINK](https://<your_account>.github.io/react_autocomplete/) and add it to the PR description.
- Replace `<your_account>` with your Github username in the [DEMO LINK](https://Nika-Andriy.github.io/react_autocomplete/) and add it to the PR description.
- Don't remove the `data-qa` attributes. It is required for tests.

## Troubleshooting
Expand Down
127 changes: 85 additions & 42 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,73 +1,116 @@
import React from 'react';
import React, { useEffect, useRef, useState } from 'react';
import './App.scss';
import { peopleFromServer } from './data/people';

export const App: React.FC = () => {
const { name, born, died } = peopleFromServer[0];
type Person = {
name: string;
born: number;
died: number;
};

type Props = {
debounceDelay?: number;
onSelected?: (person: Person) => void;
};

export const App: React.FC<Props> = ({ debounceDelay = 300, onSelected }) => {
const [query, setQuery] = useState('');
const [suggestions, setSuggestions] = useState<Person[]>(peopleFromServer);
const [selectedPerson, setSelectedPerson] = useState<Person | null>(null);
const [isOpen, setIsOpen] = useState(false);
const isSelectingRef = useRef(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.

isSelectingRef is declared but never updated. This makes the ref dead code and the intent (preventing filtering during selection) is not implemented — either set it when selecting or remove/replace this logic.


const filteredPeople = (value: string) => {
const q = value.trim().toLowerCase();

if (!q) {
return peopleFromServer;
}

return peopleFromServer.filter(person =>
person.name.toLowerCase().includes(q),
);
};

const showTitle = ({ name, born, died }: Person) =>
`${name} (${born} - ${died})`;

useEffect(() => {
if (isSelectingRef.current) {
return;
}
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 effect returns early when isSelectingRef.current is truthy, but since the ref is never set elsewhere this check is ineffective. Either set the ref around the selection operation or remove the guard and prevent redundant filtering by another means.


const timer = setTimeout(() => {
setSuggestions(filteredPeople(query));
}, debounceDelay);

return () => clearTimeout(timer);
}, [query, debounceDelay]);
Comment on lines +26 to +44
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 effect unconditionally calls setSuggestions(filteredPeople(query)) whenever query changes. This can re-run filtering even when the trimmed query hasn't actually changed (for example when only trailing spaces are typed or when the visible text is identical), which violates the requirement: "Do not run the filtering logic again if the text value has not changed since the last filtering (e.g., when there is just a pause in typing with identical text)." Consider storing the last trimmed query in a ref and skipping the filtering when the new trimmed query equals the last one, or set/reset a selection guard ref when selecting a suggestion so the effect exits early.

Comment on lines +26 to +44
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 effect schedules filtering on every query change and does not skip work when the trimmed query is unchanged. The task (and previous review) requires: keep the last trimmed query in a useRef and return early if the new trimmed query equals lastTrimmedQueryRef.current before running filtering. Consider storing the trimmed value in a ref and comparing it here to avoid redundant filtering (and avoid re-running the same filter on pauses or trailing-space edits).


return (
<div className="container">
<main className="section is-flex is-flex-direction-column">
<h1 className="title" data-cy="title">
{`${name} (${born} - ${died})`}
{selectedPerson ? showTitle(selectedPerson) : 'No selected person'}
</h1>

<div className="dropdown is-active">
<div className={`dropdown ${isOpen ? 'is-active' : ''}`}>
<div className="dropdown-trigger">
<input
type="text"
placeholder="Enter a part of the name"
className="input"
placeholder="Enter a part of the name"
data-cy="search-input"
value={query}
onFocus={() => {
setIsOpen(true);

if (!query.trim()) {
setSuggestions(peopleFromServer);
}
}}
onChange={e => {
setQuery(e.target.value);
setSelectedPerson(null);
}}
/>
</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>
{suggestions.map(person => (
<a
key={person.name}
className="dropdown-item"
data-cy="suggestion-item"
onMouseDown={() => {
setSelectedPerson(person);
setQuery(person.name);
setIsOpen(false);
onSelected?.(person);
}}
Comment on lines +84 to +90
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 the selection handler you should set isSelectingRef.current = true before updating query (and reset it to false afterwards, e.g. in a microtask or in a subsequent effect) so the debounce effect does not schedule an unnecessary filtering pass after selection. Alternatively, remove the ref and rely on comparing previous/new query values to avoid extra filtering.

>
{person.name}
</a>
))}
</div>
</div>
</div>

<div
className="
{suggestions.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>
is-align-self-flex-start"
role="alert"
data-cy="no-suggestions-message"
Comment on lines 50 to +110
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You replaced the required data-qa attributes with data-cy on several elements (e.g. data-cy="title", data-cy="search-input", data-cy="suggestions-list", data-cy="suggestion-item", data-cy="no-suggestions-message"). The task explicitly states: "Don't remove the data-qa attributes. It is required for tests." Restore the exact data-qa attributes/values (e.g. data-qa="title", data-qa="search-input", etc.) because automated tests rely on them.

Comment on lines 50 to +110
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Replace these data-cy attributes with the required data-qa attributes so automated tests can find elements. Update the title, search input, suggestions list, suggestion item, and no-suggestions message attributes (e.g. data-cy="title" -> data-qa="title", etc.). The task explicitly says: "Don't remove the data-qa attributes; they are required for tests."

>
<p className="has-text-danger">No matching suggestions</p>
</div>
)}
</main>
</div>
);
Expand Down
Loading