Solution#1172
Conversation
| import { Person } from '../../types/Person'; | ||
|
|
||
| type Prop = { | ||
| visible: boolean; |
There was a problem hiding this comment.
it's common practice to name boolean consts, props, arguments using prefixes like has, is, are. e.g. hasAccess, areEquals etc
| onClick={() => onSelected(item)} | ||
| > | ||
| <p | ||
| className={`${item.sex === 'f' ? 'has-text-danger' : 'has-text-link'}`} |
There was a problem hiding this comment.
use classnames for conditional classes
| import { Dropdown } from './components/Dropdown/Dropdown'; | ||
| import { useDebounce } from './hooks/useDebounce'; | ||
|
|
||
| function filteredPerson(people: Person[], query: string): Person[] { |
There was a problem hiding this comment.
bad function naming, function names usually should indicate an action. in your case it's named like it is a filtered person and not an action
| function filteredPerson(people: Person[], query: string): Person[] { | ||
| const queryLowerCase = query.trim().toLowerCase(); | ||
|
|
||
| if (query !== '') { |
There was a problem hiding this comment.
i assume the check should be queryLowerCase !== '' instead.
in your current functionality you are trimming the queryLowerCase but checking query which means that if query has empty spaces like ' ' it would be valid for if condition
| people: Person[]; | ||
| onSelected: (person: Person) => void; | ||
| }; | ||
| export const Dropdown = memo(function Dropdown({ |
There was a problem hiding this comment.
it looks like unnecessary memoization
| onChange={e => { | ||
| setSearch(e.target.value); | ||
| if (selectedPerson) { | ||
| setSelectedPerson(null); | ||
| } | ||
| }} |
There was a problem hiding this comment.
inline event handlers that exceeds one line should be moved into named handlers
2 participants
DEMO LINK