Added skeleton loader on slow machinery requests#4086
Added skeleton loader on slow machinery requests#4086wassafshahzad wants to merge 5 commits intomozilla:mainfrom
Conversation
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
|
The missing lines in test, are in MachineryTranslations.tsx around the async fetch logic. That code was already there before my changes, I just wired fetching state into it. Testing it properly would need mocking 5 API calls, Redux, and multiple React contexts which felt out of scope for a loading indicator fix. Added two tests in Machinery.test.jsx covering the actual UI change instead. Happy to add provider tests in a follow-up if needed. |
mathjazz
left a comment
mathjazz
left a comment
There was a problem hiding this comment.
Thanks for the patch!
Please see my comment below for the SkeletonLoader layout.
| ))} | ||
| </ul> | ||
| {machineryFetching && translations.length === 0 && ( | ||
| <SkeletonLoader items={[]} /> |
There was a problem hiding this comment.
This is a good opportunity to introduce a new skeleton loader, that matches the layout of Machinery results. The one we use in this file is actually imitating the string list results.
There was a problem hiding this comment.
Is this UI fine ?
There was a problem hiding this comment.
Thanks for the update! Could you please separate the lines for source and target text, and make the full-width and same height as they were in the SkeletonLoader?
mathjazz
left a comment
mathjazz
left a comment
There was a problem hiding this comment.
Nice work - it works really well!
Please see my comment inline for improved code maintainability.
| @@ -0,0 +1,33 @@ | |||
| import React from 'react'; | |||
|
|
|||
| import '~/modules/loaders/components/SkeletonLoader.css'; | |||
There was a problem hiding this comment.
This file should be moved to translate/src/modules/loaders/components and have it's own MachinerySkeletonLoader.tsx file.
3 participants
Description
Added a skeleton loader and fetch state to render skeleton loader on slow machinery requests.
Linked Issue
Closes #4074
Screenshot
Skeleton Machinery Loader
Skeleton Machinery Loader Updated UI