-
Notifications
You must be signed in to change notification settings - Fork 55
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
fix(RHINENG-11976): fix loading states across app #1300
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1300 +/- ##
==========================================
- Coverage 22.22% 22.19% -0.04%
==========================================
Files 96 96
Lines 2560 2564 +4
Branches 811 811
==========================================
Hits 569 569
- Misses 1991 1995 +4 ☔ View full report in Codecov by Sentry. |
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.
This is a step in the right direction, I added some suggestions; I'd say the ideal state is to remove the Loading
component since it's super ugly and replace all occurrences of SkeletonTable from FEC to PF component groups. This can be done in a single or multiple PRs.
a03d31d
to
4a754ff
Compare
a58a349
to
9125a1f
Compare
9712076
to
5f5c5bd
Compare
/retest |
ff4d7be
to
359716e
Compare
/retest |
Nice improvements, I just added some minor comments to fix. |
f7c7798
to
843ad04
Compare
package-lock.json
Outdated
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.
I am surprised these massive changes to package-lock.json are really necessary. As I can see from package.json, we have only added the @patternfly/react-component-groups library. Could you please double-check if dependencies are updated correctly?
9ec29c9
to
8e4e330
Compare
I agree with Georgii, package.json and package-lock.json do not correspond to one another -- the easiest fix would be to revert changes to both files and install |
Other than that the PR is good to be merged IMO. |
8e4e330
to
dfe9128
Compare
dfe9128
to
0c14291
Compare
Description
Fix loading states across app to better reflect loading components
Associated Jira ticket: RHINENG-11976
How to test the PR
Affected pages:
/recommendations
- Pathways tab/recommendation/:id
- loading spinner while the tables in the tabs are loading/topics
- table/topics/:id
- rules table/systems/:id
- header, tableChecklist: