Develop #609
Develop #609
Conversation
src/components/PeoplePage.tsx
Outdated
| .finally(() => setIsLoad(false)); | ||
| }, []); | ||
|
|
||
| const getVisiblePeople = () => { |
There was a problem hiding this comment.
Better move this function outside the component and just call it here with your arguments.
| setIsLoad(true); | ||
| getPeople() | ||
| .then((peopleFromServer) => setPeople(peopleFromServer)) | ||
| .catch(() => { |
There was a problem hiding this comment.
Try to catch an error here, maybe it will fix some of your failed tests
src/components/PeoplePage.tsx
Outdated
|
|
||
| export const PeoplePage = () => { | ||
| const [people, setPeople] = useState<Person[] | null>(null); | ||
| const [isLoad, setIsLoad] = useState<boolean>(false); |
There was a problem hiding this comment.
It is better to make isLoad true by default. Now there's no need to set it true in line 23
| </p> | ||
| )} | ||
| {people && <PeopleTable people={getVisiblePeople()} />} | ||
| {!isLoad && !error && !people && ( |
There was a problem hiding this comment.
no people appears before page is loaded for a moment. Create a variable for this and try to change your people condition.
There was a problem hiding this comment.
Maybe I've already fixed it with lowder, because I don't see this problem now
src/components/PeoplePage.tsx
Outdated
|
|
||
| useEffect(() => { | ||
| getPeople() | ||
| .then((peopleFromServer) => setPeople(peopleFromServer)) |
src/components/PeoplePage.tsx
Outdated
| people={getVisiblePeople( | ||
| people, | ||
| sex, | ||
| query, | ||
| centuries, | ||
| sort, | ||
| order, | ||
| )} |
There was a problem hiding this comment.
- For better readability store in separate variable
- Just pass the whole
searchParamsobject
src/components/PeopleFilters.tsx
Outdated
| <SearchLink | ||
| className={classnames({ 'is-active': !sex })} | ||
| params={{ sex: null }} | ||
| > | ||
| All | ||
| </SearchLink> | ||
|
|
||
| <SearchLink | ||
| className={classnames({ 'is-active': sex === 'm' })} | ||
| params={{ sex: 'm' }} | ||
| > | ||
| Male | ||
| </SearchLink> | ||
|
|
||
| <SearchLink | ||
| className={classnames({ 'is-active': sex === 'f' })} | ||
| params={{ sex: 'f' }} | ||
| > | ||
| Female | ||
| </SearchLink> |
| className="button is-link is-outlined is-fullwidth" | ||
| href="#/people" | ||
| params={{ centuries: null, query: null, sex: null }} |
There was a problem hiding this comment.
Find a better way to reset all by just 1 line of code. If you had 20 filters, would you set null to all of them?
There was a problem hiding this comment.
all the other options I can think of
will either just increase the amount of code or reset the sorting as well.
DEMO LINK