-
Notifications
You must be signed in to change notification settings - Fork 44
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
Feat/discovery load time spike #1579
base: master
Are you sure you want to change the base?
Conversation
…with query params, css to gracefully hide them
…r, changed a boolean to an object
…s bar loading rate
…se a useEffect instead of combination of CSS transitions
…ss bar loading and dismissal more smooth
…will use mini version. Added functionality for loading spinners for Data Repository and Filters
…ed; removed disabling logic
src/Discovery/index.tsx
Outdated
// to improve load time & usability | ||
// Initially uses a smaller batch to load interface quickly | ||
// Then a batch with all the studies | ||
const [studiesBatchCount, setStudiesBatchCount] = useState(0); |
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.
Note: Update the variable name to "numberOfBatchesLoaded"
src/Discovery/index.tsx
Outdated
@@ -167,16 +202,27 @@ const DiscoveryWithMDSBackend: React.FC<{ | |||
|
|||
// indicate discovery tag is active even if we didn't click a button to get here | |||
props.onDiscoveryPageActive(); | |||
}, []); | |||
}, [props, studiesBatchCount]); |
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.
Note: Update the variable name to "numberOfBatchesLoaded"
src/Discovery/index.tsx
Outdated
useEffect(() => { | ||
// If batch loading is Enabled, update the studiesBatchCount to enable calling of different batch sizes | ||
// with different parameters | ||
if (studiesBatchCount < expectedNumberOfTotalBatches) setStudiesBatchCount(studiesBatchCount + 1); |
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.
Note: Update the variable name to "numberOfBatchesLoaded"
…mberOfBatchesLoaded to improve clarity
Please find the ci env pod logs here |
Spike for Improve Discovery Page Loading UX
Ticket: https://ctds-planx.atlassian.net/browse/HP-1643
Before
Load time is approximately 5 seconds. Searches entered during loading do not update once loading is completed.
After
UX proposed with batch loading of 10 studies and then the rest. Initial load of first 10 studies takes around 200-300ms (95% improvement). UX shown includes a mini progress bar.
Instructions:
Pull local, then run in QA Dev.
Mini Progress Bar by Studies Count:
https://qa-heal.planx-pla.net/portal/dev.html?showMini