-
Notifications
You must be signed in to change notification settings - Fork 1.3k
add task solution #1155
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
base: master
Are you sure you want to change the base?
add task solution #1155
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,56 @@ | ||
| import React from 'react'; | ||
| import './App.scss'; | ||
| import { peopleFromServer } from './data/people'; | ||
| import { Person } from './types/Person'; | ||
| import React, { useCallback, useState } from 'react'; | ||
| import debounce from 'lodash.debounce'; | ||
|
|
||
| export const App: React.FC = () => { | ||
| const { name, born, died } = peopleFromServer[0]; | ||
| type Props = { | ||
| persons: Person[]; | ||
| delay?: number; | ||
| onSelected?: (person: Person) => void; | ||
| }; | ||
|
|
||
| export const App: React.FC<Props> = ({ persons, delay = 300, onSelected }) => { | ||
| const [query, setQuery] = useState(''); | ||
| const [appliedQuery, setAppliedQuery] = useState(''); | ||
| const [selectedPerson, setSelectedPerson] = useState<Person | null>(null); | ||
| const [isFocused, setIsFocused] = useState(false); | ||
|
|
||
| const applyQuery = useCallback(debounce(setAppliedQuery, delay), [delay]); | ||
|
|
||
| React.useEffect(() => { | ||
| return () => { | ||
| applyQuery.cancel?.(); | ||
| }; | ||
| }, [applyQuery]); | ||
|
|
||
| const handleQueryChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| const value = event.target.value; | ||
|
|
||
| setQuery(value); | ||
| setSelectedPerson(null); | ||
|
|
||
| const trimmed = value.trim(); | ||
|
|
||
| if (!trimmed) { | ||
| setAppliedQuery(''); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| applyQuery(trimmed); | ||
|
Comment on lines
+31
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The handler always calls the debounced updater for any non-empty trimmed value. This violates the requirement “don't run filtering again if the text has not changed (a pause in typing happened when the text was the same as before)”. Add a check comparing the new trimmed value to the current |
||
| }; | ||
|
|
||
| const filteredPeople = persons.filter(person => | ||
| person.name.toLowerCase().includes(appliedQuery.trim().toLowerCase()), | ||
| ); | ||
|
|
||
| return ( | ||
| <div className="container"> | ||
| <main className="section is-flex is-flex-direction-column"> | ||
| <h1 className="title" data-cy="title"> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The task and tests require |
||
| {`${name} (${born} - ${died})`} | ||
| {selectedPerson | ||
| ? `${selectedPerson.name} (${selectedPerson.born} - ${selectedPerson.died})` | ||
| : 'No selected person'} | ||
| </h1> | ||
|
|
||
| <div className="dropdown is-active"> | ||
|
|
@@ -19,55 +60,55 @@ | |
| placeholder="Enter a part of the name" | ||
| className="input" | ||
| data-cy="search-input" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The input uses |
||
| value={query} | ||
| onChange={handleQueryChange} | ||
| onFocus={() => setIsFocused(true)} | ||
| onBlur={() => setIsFocused(false)} | ||
| /> | ||
| </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> | ||
| {isFocused && filteredPeople.length > 0 && ( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The suggestions list visibility depends on |
||
| <div | ||
| className="dropdown-menu is-active" | ||
| role="menu" | ||
| data-cy="suggestions-list" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The suggestions list uses |
||
| > | ||
| <div className="dropdown-content"> | ||
| {filteredPeople.map(person => ( | ||
| <div | ||
| key={person.name} | ||
| className="dropdown-item" | ||
| data-cy="suggestion-item" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each suggestion item uses |
||
| onMouseDown={() => { | ||
| setSelectedPerson(person); | ||
| setQuery(person.name); | ||
| setAppliedQuery(person.name); | ||
| onSelected?.(person); | ||
| }} | ||
|
Comment on lines
+86
to
+92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When a suggestion is selected you set the input value and |
||
| > | ||
| <p className="has-text-link">{person.name}</p> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
|
|
||
| <div | ||
| className=" | ||
| {(appliedQuery || query) && filteredPeople.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> | ||
| role="alert" | ||
| data-cy="no-suggestions-message" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The no-results notification uses |
||
| > | ||
| <p className="has-text-danger">No matching suggestions</p> | ||
| </div> | ||
| )} | ||
|
Comment on lines
101
to
+116
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The ‘no suggestions’ message visibility uses |
||
| </main> | ||
| </div> | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The debounced updater is created with
useCallback(debounce(setAppliedQuery, delay), [delay])but there is no cleanup to cancel pending debounced calls on unmount or whendelaychanges. It is safer to create the debounced function inside auseMemo/useEffectand call.cancel()in cleanup to avoid possible stale calls or memory leaks.