-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Develop #511
base: master
Are you sure you want to change the base?
Develop #511
Conversation
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.
Good job, but there is quite a bit of work to be done.
There are also currently several problems with the application itself:
- When switching to the
People page
, both the loader and the people data will be renedred at the same time:
https://gyazo.com/d715d2c8a2717835cc280ab49675c0e9 - Sorting does not work:
https://gyazo.com/a87eb63dfd784c296bee21703737795c
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.
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.
#511 (review) - not fixed 😢
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.
Good work 👍
But, get rid of setTimeout as I mentioned in the comment
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.
setTimeout(async () => { | ||
try { | ||
const peopleFromServer = await getPeople(); | ||
|
||
setPeople(getPreparedPeople(peopleFromServer)); | ||
} catch (error) { | ||
setIsError(true); | ||
} finally { | ||
setIsLoading(false); | ||
} | ||
}, 1000); |
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.
Why do you fetch
in the setTimeout
callback?
It doesn't make any sense. Get rid of a timeout
DEMO LINK