solution#1159
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This implementation has two critical blockers that prevent task completion. First, the delay prop is missing — the task requires this to be customizable via props with a default of 300ms, but your component uses an internal useState(300) instead of accepting it as a prop. Second, the onSelected callback is not being invoked — you have a state variable named onSelected which shadows the prop callback, and you're setting internal state instead of calling the passed callback function with the selected person. To fix: add a props interface with delay?: number (default 300) and onSelected: (person: Person) => void, and call props.onSelected(person) when a suggestion is clicked rather than updating internal state. These are explicit requirements from the task that must be implemented for the component to work as specified.
✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.
Footnotes
| import React, { useMemo, useState } from 'react'; | ||
| import './App.scss'; | ||
| import { peopleFromServer } from './data/people'; | ||
| import debounce from 'lodash.debounce'; | ||
| import { Person } from './types/Person'; | ||
|
|
||
| export const App: React.FC = () => { | ||
| const { name, born, died } = peopleFromServer[0]; | ||
| const [query, setQuery] = useState(''); | ||
| const [appliedQuery, setAppliedQuery] = useState(''); | ||
| const [delay] = useState(300); |
There was a problem hiding this comment.
The delay prop is missing. The task requires delay to be customizable via props with default 300ms, but this component has no props interface and no delay parameter.
| import React, { useMemo, useState } from 'react'; | ||
| import './App.scss'; | ||
| import { peopleFromServer } from './data/people'; | ||
| import debounce from 'lodash.debounce'; | ||
| import { Person } from './types/Person'; | ||
|
|
||
| export const App: React.FC = () => { | ||
| const { name, born, died } = peopleFromServer[0]; | ||
| const [query, setQuery] = useState(''); | ||
| const [appliedQuery, setAppliedQuery] = useState(''); | ||
| const [delay] = useState(300); | ||
| const [onSelected, setOnSelected] = useState<Person | null>(null); |
There was a problem hiding this comment.
The task requires passing selected person to onSelected callback passed via props. Currently using internal state setOnSelected instead of accepting and calling an onSelected prop callback function.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation now correctly addresses all the critical requirements from the previous review. The delay prop is properly implemented with a default of 300ms, and the onSelected callback is correctly invoked without being shadowed by internal state. All functional requirements are met including debounced filtering, Bulma dropdown styling, "No matching suggestions" messaging, and proper handling of focus/selection states. One minor note: there's a typo in the import path where src/commponents/Dropdown/Dropdown.tsx should be src/components/Dropdown/Dropdown.tsx. While this doesn't break functionality on macOS/Linux, you may want to rename the directory to maintain consistency with React conventions and avoid potential issues in other environments.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| import { Person } from '../../types/Person'; | ||
| import debounce from 'lodash.debounce'; | ||
|
|
||
| interface Props { |
There was a problem hiding this comment.
Typo in import path: 'commponents' should be 'components'
2 participants
No description provided.