Skip to content
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

FI-2405: Create skeleton loader for main app #440

Merged
merged 16 commits into from Jan 24, 2024

Conversation

AlyssaWang
Copy link
Collaborator

Summary

Adds skeleton loading components to the test session portion of Inferno. Follow up tickets will implement skeletons for the landing page and suite options page.

Testing Guidance

Screenshot 2024-01-23 at 8 04 51 PM

Skeletons should appear while portions of the application are loading. Please let me know if they aren't doing so!

Copy link

codecov bot commented Jan 24, 2024

Codecov Report

Attention: 58 lines in your changes are missing coverage. Please review.

Comparison is base (58320f3) 77.09% compared to head (39d4f80) 77.32%.

Files Patch % Lines
...nt/src/components/TestSuite/TestSessionWrapper.tsx 30.23% 30 Missing ⚠️
client/src/components/Skeletons/FooterSkeleton.tsx 83.67% 2 Missing and 6 partials ⚠️
client/src/components/Skeletons/HeaderSkeleton.tsx 86.04% 1 Missing and 5 partials ⚠️
client/src/components/Skeletons/DrawerSkeleton.tsx 89.13% 1 Missing and 4 partials ⚠️
...t/src/components/Skeletons/TestSessionSkeleton.tsx 92.42% 1 Missing and 4 partials ⚠️
client/src/components/Skeletons/AppSkeleton.tsx 90.62% 0 Missing and 3 partials ⚠️
client/src/components/Header/Header.tsx 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #440      +/-   ##
==========================================
+ Coverage   77.09%   77.32%   +0.22%     
==========================================
  Files         220      225       +5     
  Lines       10951    11200     +249     
  Branches     1030     1052      +22     
==========================================
+ Hits         8443     8660     +217     
- Misses       1929     1939      +10     
- Partials      579      601      +22     
Flag Coverage Δ
backend 93.92% <ø> (ø)
frontend 70.15% <79.43%> (+0.55%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@arscan arscan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huge improvement! I tried it out with our demo suites, and also g10 after running the full suite of single patient api tests, which causes the interface load time to be a bit long right now. A todo for us is to optimize some of our db calls to improve load time, but this is a nice short term measure for that, plus being noticeably better in the regular case.

My only concern here is that it would add enough complexity and/or maintenance effort when we update the look and feel to make it simply not worthwhile to put in. But in looking through the code, it looks like there are very few spots in the 'real' interface that link over to this skeleton (so, low complexity), and the skeleton API seems really simple to update (so, low enough maintenance cost for if/when we update the UI). So I definitely think its worthwhile.

I'm not so concerned about skeletons on the landing/suites part of the UI, as that has a decent likelyhood of being refactored down the road as we consider backporting jekyll updates we made to inferno.healthit.gov into the test kit template.

@arscan
Copy link
Contributor

arscan commented Jan 24, 2024

I did see a bit of a flicker for suites with options due to some kind of reloading going on, which might be a little more annoying now that this page isn't just a grey blob, but I think we are aware of that issue.

@AlyssaWang AlyssaWang merged commit 54713d5 into main Jan 24, 2024
10 checks passed
@AlyssaWang AlyssaWang deleted the FI-2405-skeleton-loader-main branch January 24, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants