-
Notifications
You must be signed in to change notification settings - Fork 248
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
BREAKING: Merge vitals and biometrics apps #1429
Conversation
Essentially, this merges the biometrics app into the vitals app. There isn't really a strong reason to separate them and their implementation details already had enough overlap that we were effectively requiring the same configuration settings to be set for both apps. Finally, its cleaner to get everything to work with SWR caching if certain implementation details are shared, so I've collapsed them into a single application.
8797cbc
to
ba5b1fe
Compare
Size Change: -758 kB (-8%) ✅ Total Size: 8.65 MB
ℹ️ View Unchanged
|
Hi @ibacher, question about this work: What would be the implications of this for sites that don't actually want the biometrics app to show up everywhere the Vitals app does? E.g. IIRC at some sites, like Ampath, they wanted the VS chart on the pt summary, but not the Biometrics table. Do implementers lose that configurability? |
No. They have the same configurability options. The biometrics table can easily be removed using the config system, which remains our preferred mechanism for implementations to opt out of functionality. (As long as the biometrics table is configured to not show, there's no substantive performance impact to having the bundle avaiable). |
@denniskigen I'd appreciate a review here when you get the chance. I'd like to get this in before replacing the |
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.
Excellent work, @ibacher. LGTM!
Requirements
Summary
Essentially, this merges the biometrics app into the vitals app. The main motivation is to clean-up the shared configurations and SWR caching.
Screenshots
Related Issue
Other