Done by pivkota · Pull Request #1133 · mate-academy/react_autocomplete

pivkota · 2026-03-08T12:53:00Z

mateacademy-ai-mentor

Review Summary

I am requesting changes for this submission. You've done a great job implementing the core features like debouncing and filtering suggestions. The main issue is that the task requires creating a reusable Autocomplete component, but all the logic is currently implemented directly within App.tsx.

This architectural choice leads to a few critical requirements being missed:

Missing Props: The component needs to accept props for a customizable delay and an onSelected callback function. Currently, the delay is hardcoded, and there is no callback.
Filtering on Spaces: The filter runs when the input contains only spaces, which should be prevented according to the checklist [CHECKLIST ITEM #1].

Refactoring your code into a separate Autocomplete component that accepts these props will address the core issues. You're very close to a complete solution!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-03-08T12:55:35Z

src/App.tsx

+  const [query, setQuery] = useState('');
+  const [appliedQuery, setAppliedQuery] = useState('');
+  const [showField, setShowField] = useState(false);
+  const [selectdPerson, setSelectdPerson] = useState<Person | null>(null);


There's a typo in the state variable name. It should be selectedPerson.

mateacademy-ai-mentor · 2026-03-08T12:55:35Z

src/App.tsx

+  const [appliedQuery, setAppliedQuery] = useState('');
+  const [showField, setShowField] = useState(false);
+  const [selectdPerson, setSelectdPerson] = useState<Person | null>(null);
+  const timeoutQuery = useMemo(() => debounce(setAppliedQuery, 300), []);


The debounce delay is hardcoded to 300ms. According to the task description, this delay should be customizable via props with a default value of 300ms. This would be possible if the logic were in a separate Autocomplete component.

mateacademy-ai-mentor · 2026-03-08T12:55:35Z

src/App.tsx

+  const peopleToShow = peopleFromServer.filter(person => {
+    const nameLower = person.name.toLowerCase().trim();
+    const queryLower = appliedQuery.toLowerCase().trim();
+
+    return nameLower.includes(queryLower);
+  });


The filtering logic to calculate peopleToShow runs on every render of the App component. This is inefficient and violates the requirement to not run filtering again if the text has not changed. You should memoize this calculation using the useMemo hook, with appliedQuery as a dependency.

mateacademy-ai-mentor · 2026-03-08T12:55:35Z

src/App.tsx

+        <div
+          className={classNames('dropdown', {
+            'is-active': showField && peopleToShow.length !== 0,
+          })}
+        >


The dropdown list doesn't close when the user clicks outside of it (loses focus). This can be a poor user experience. Consider adding an onBlur event handler to the dropdown's container to set showField to false.

mateacademy-ai-mentor · 2026-03-08T12:55:35Z

src/App.tsx

+                  onClick={() => {
+                    setSelectdPerson(person);
+                    setShowField(false);
+                    setQuery(person.name);
+                  }}


When a suggestion is clicked, the selected person object should be passed to an onSelected callback function provided via props, as required by the task description.

mateacademy-ai-mentor

Review Summary

I am requesting changes. You've made good progress by adding the delay and onSelected props, which is great to see! However, two main issues from the previous review still need to be addressed. First, the primary goal is to create a separate, reusable Autocomplete component. All related logic, state, and JSX should be moved out of App.tsx and into a new Autocomplete.tsx file. Second, the filter still runs when the input contains only spaces, which violates a checklist requirement [CHECKLIST ITEM #1]. Please add a check to prevent this.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-03-08T13:03:30Z

src/App.tsx

+  onSelected?: (person: Person) => void;
+}
+
+export const App: React.FC<AutocompleteProps> = ({


The task requires you to create a reusable Autocomplete component. While you've added the required props (delay, onSelected), all the logic is still inside the App component. The goal is to separate the autocomplete functionality into its own component file (e.g., src/Autocomplete.tsx) which can then be used within App.tsx. This was the main point from the previous review.

mateacademy-ai-mentor · 2026-03-08T13:03:30Z

src/App.tsx

+
+  const handleInput = (e: React.ChangeEvent<HTMLInputElement>) => {
+    setQuery(e.target.value);
+    timeoutQuery(e.target.value);


This violates checklist item #1: 'make sure that filter won't be called if user entered spaces only'. Currently, when the user types only spaces, the debounced update is still triggered. This causes the filter to run and, because appliedQuery.trim() becomes an empty string, it shows all suggestions. You should add a check to avoid running the filter when the input contains only whitespace.

mateacademy-ai-mentor

Review Summary

Great work on this iteration! You've successfully refactored the logic into a separate Autocomplete component and fixed the issue with space-only input filtering. That's excellent progress.

The main area for improvement is to fully separate the component's responsibilities. The Autocomplete component is currently managing the selectedPerson state and displaying it in an <h1>. To make the component truly reusable and meet the task requirements, this state and the <h1> should be moved to the App component. This is a common and important pattern in React called "lifting state up."

I am approving this submission as an educational override to help you progress, but please apply this feedback in your next steps. Keep up the great work!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2026-03-08T13:27:59Z

src/App.tsx

-          <p className="has-text-danger">No matching suggestions</p>
-        </div>
+      <main className="section">
+        <Autocomplete data={peopleFromServer} delay={300} />


The task requires the h1 element for displaying the selected person to be located here in the App component, likely after the Autocomplete component. To make this work, you'll need to manage the selectedPerson state within App.tsx and use the onSelected prop to get the selected person from the Autocomplete component.

mateacademy-ai-mentor · 2026-03-08T13:27:59Z

src/components/Autocomplete.tsx

+  const [query, setQuery] = useState('');
+  const [appliedQuery, setAppliedQuery] = useState('');
+  const [showField, setShowField] = useState(false);
+  const [selectedPerson, setSelectedPerson] = useState<Person | null>(null);


This state should be managed by the parent component (App.tsx) to make the Autocomplete component more reusable. The Autocomplete component's responsibility is to report which person was selected, not to store that selection itself.

mateacademy-ai-mentor · 2026-03-08T13:27:59Z

src/components/Autocomplete.tsx

+      setAppliedQuery('');
+    }
+
+    setSelectedPerson(null);


Instead of setting the local state here, you should use the onSelected prop to inform the parent component that the selection has been cleared. You may need to adjust the type of the onSelected prop to accept null as a value.

mateacademy-ai-mentor · 2026-03-08T13:27:59Z

src/components/Autocomplete.tsx

+      <h1 className="title" data-cy="title">
+        {selectedPerson
+          ? `${selectedPerson.name} (${selectedPerson.born} - ${selectedPerson.died})`
+          : 'No selected person'}
+      </h1>


This <h1> element should be in the App component as per the task requirements: "add an h1 to the App showing ...". The Autocomplete component should not be responsible for displaying the selected result; it should only provide the selection functionality.

Done

86ba84e

mateacademy-ai-mentor suggested changes Mar 8, 2026

View reviewed changes

Fix

e6c3085

pivkota requested a review from mateacademy-ai-mentor March 8, 2026 13:01

mateacademy-ai-mentor suggested changes Mar 8, 2026

View reviewed changes

Fixed

a66c5d6

pivkota requested a review from mateacademy-ai-mentor March 8, 2026 13:26

mateacademy-ai-mentor approved these changes Mar 8, 2026

View reviewed changes

Conversation

pivkota commented Mar 8, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants