-
Notifications
You must be signed in to change notification settings - Fork 1.3k
solution #1158
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?
solution #1158
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import React, { useEffect, useState } from 'react'; | ||
| import { Person } from '../types/Person'; | ||
| import { useFilter } from '../hooks/Filter'; | ||
| import { PeopleList } from './peopleList'; | ||
|
|
||
| interface Props { | ||
| people: Person[]; | ||
| delay?: number; | ||
| onSelected: (person: Person | null) => void; | ||
| } | ||
|
|
||
| export const Autocomplete: React.FC<Props> = ({ | ||
| people, | ||
| delay = 300, | ||
| onSelected, | ||
| }) => { | ||
| const [rawInput, setRawInput] = useState(''); | ||
| const [query, setQuery] = useState(''); | ||
| const [show, setShow] = useState(false); | ||
|
|
||
| const filteredPeople = useFilter(people, query); | ||
|
|
||
| // 🔹 debounce | ||
| useEffect(() => { | ||
| const trimmed = rawInput.trim(); | ||
|
|
||
| const timer = setTimeout(() => { | ||
| setQuery(trimmed); // даже пустая строка → покажет всех | ||
|
Comment on lines
+24
to
+28
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. This violates checklist item #1: "make sure that filter won't be called if user enred spaces only;" In Autocomplete.tsx the debounce effect sets |
||
| }, delay); | ||
|
|
||
| return () => clearTimeout(timer); | ||
|
Comment on lines
+24
to
+31
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. This violates the checklist instruction: "don't run filtering again if the text has not changed (a pause in typing happened when the text was the same as before)". The current debounce effect depends only on [rawInput, delay] and always sets |
||
| }, [rawInput, delay]); | ||
|
|
||
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const value = e.target.value; | ||
|
|
||
| setRawInput(value); | ||
| setShow(true); | ||
|
|
||
| // 🔴 ВАЖНО: сбрасываем выбранного человека | ||
| onSelected(null); | ||
|
Comment on lines
+34
to
+41
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. Small issue related to Checklist: Although you reset selected person via |
||
| }; | ||
|
|
||
| const handleSelect = (person: Person) => { | ||
| setRawInput(person.name); | ||
| setQuery(person.name); | ||
| setShow(false); | ||
| onSelected(person); | ||
| }; | ||
|
|
||
| const showError = show && query !== '' && filteredPeople.length === 0; | ||
|
|
||
| return ( | ||
| <div className={`dropdown ${show ? 'is-active' : ''}`}> | ||
| <div className="dropdown-trigger"> | ||
| <input | ||
| type="text" | ||
| className="input" | ||
| data-cy="search-input" | ||
| placeholder="Enter a part of the name" | ||
| value={rawInput} | ||
| onChange={handleChange} | ||
| onFocus={() => setShow(true)} | ||
| /> | ||
| </div> | ||
|
|
||
| {show && ( | ||
| <div className="dropdown-menu" data-cy="suggestions-list"> | ||
| <div className="dropdown-content"> | ||
| <PeopleList people={filteredPeople} onSelect={handleSelect} /> | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| {showError && ( | ||
| <div | ||
| className="notification is-danger is-light mt-2" | ||
| data-cy="no-suggestions-message" | ||
| > | ||
| No matching suggestions | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import type { Person } from '../types/Person'; | ||
|
|
||
| interface PeopleListProps { | ||
|
Comment on lines
+1
to
+3
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. Checklist item: "Don't interact with DOM directly, use React as much as possible" — code uses React state and props, no direct DOM manipulation found. Satisfied. |
||
| people: Person[]; | ||
| onSelect?: (person: Person) => void; | ||
| } | ||
|
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. This file doesn't prevent filtering when the input contains only spaces. The checklist requires: "make sure that filter won't be called if user enred spaces only". In |
||
|
|
||
| export const PeopleList = ({ | ||
| people, | ||
| onSelect = () => {}, | ||
| }: PeopleListProps) => { | ||
| return ( | ||
| <> | ||
| {people.map(person => ( | ||
|
Comment on lines
+1
to
+14
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's debounce requirement and customizable |
||
| <div | ||
| key={person.slug} | ||
|
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. Checklist item: "Don't generate key on render" — PeopleList uses 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. Checklist item: "don't generate key on render" is satisfied (you use stable |
||
| className="dropdown-item has-text-link is-clickable" | ||
| data-cy="suggestion-item" | ||
| onClick={() => onSelect(person)} | ||
| > | ||
| {person.name} | ||
| </div> | ||
| ))} | ||
|
Comment on lines
+22
to
+23
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. Requirement: "Show |
||
| </> | ||
| ); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import { useMemo } from 'react'; | ||
|
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. This violates checklist item #2: 'don't interact with DOM directly, use React as much as possible'. The implementation doesn't interact with the DOM directly, so no issue here (no direct DOM access found). |
||
| import { Person } from '../types/Person'; | ||
|
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. Missing requirement: 'Implement an |
||
|
|
||
|
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. Requirement: 'The |
||
| export const useFilter = (people: Person[], query: string) => { | ||
| return useMemo(() => { | ||
| if (!query.trim()) { | ||
|
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. This violates checklist item #1: 'make sure that filter won't be called if user enred spaces only'. The current filter hook calls |
||
| return people; // 👈 show all when empty | ||
| } | ||
|
|
||
| return people.filter(person => | ||
| person.name.toLowerCase().includes(query.toLowerCase()), | ||
| ); | ||
| }, [people, query]); | ||
| }; | ||
|
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. Missing debounce requirement: The description requires suggestions appear after some 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. Missing 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)'. There's no logic preventing repeated filtering on identical query pauses — currently useFilter depends on |
||
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.
Checklist item: "don't generate key on render" — in peopleList.tsx the list uses
key={person.slug}, which is stable and correct. No action needed here. (No violating pattern found.)