(feat) Add support for configurable form sections #1406
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CC: @icrc-loliveira: Note that this isn't exactly equivalent to #814; inter alia this doesn't add support for filtering forms by encounterType, though that should be straight-forward to add. I also didn't implement |
ibacher
commented
Oct 10, 2023
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "extends": "../../tsconfig.json", | |||
| "include": ["src/**/*"], | |||
| "include": ["src/**/*", "../../tools/setupTests.ts"], | |||
There was a problem hiding this comment.
@denniskigen I did this to allow TS to recognise the types from @testing-library/jest-dom. I think we talked about that once before. I think this is safe to do, as we don't currently use tsconfig to for actual compilation, just type-checking. If / when we do, we may need some scheme with a tsconfig.test.json, but that seems more complicated.
There was a problem hiding this comment.
I agree. The existing workaround (needing to launch the test-helpers.tsx file to get the IDE to load types) is too convoluted. I've not found a better approach.
|
Size Change: +38.5 kB (0%) Total Size: 9.1 MB
ℹ️ View Unchanged
|
ibacher
commented
Oct 10, 2023
| patient: fhir.Patient; | ||
| patientUuid: string; | ||
| completedForms?: Array<CompletedFormInfo>; | ||
| error?: any; |
There was a problem hiding this comment.
I hate using any, but this is meant to be directly from SWR and that's the type it gives.
There was a problem hiding this comment.
Error usually works for me.
ibacher
commented
Oct 10, 2023
| export function useForms(patientUuid: string, startDate?: Date, endDate?: Date, cachedOfflineFormsOnly = false) { | ||
| const { htmlFormEntryForms } = useConfig() as ConfigObject; | ||
| // December 31, 1969; hopefully we don't have encounters before that | ||
| const MINIMUM_DATE = new Date(0); |
There was a problem hiding this comment.
This is probably the biggest hack in this PR. The two minimum values I know we can get for dates are this (the Unix epoch) and new Date(0, 0, 0) (1900). Unix epoch seemed safe enough.
|
Wow, beautiful work Ian!! Thanks for this PR 💐 |
denniskigen
reviewed
Oct 19, 2023
packages/esm-patient-forms-app/src/forms/forms-dashboard.component.tsx
Outdated
Show resolved
Hide resolved
denniskigen
reviewed
Oct 19, 2023
|
|
||
| const handleSearch = React.useMemo(() => debounce((searchTerm) => setSearchTerm(searchTerm), 300), []); | ||
| return fuzzy |
There was a problem hiding this comment.
I like this fuzzy search approach. Perhaps we should use it in most other places.
There was a problem hiding this comment.
I got the idea from @brandones... I like that library a lot; it's simple and to the point. I'm not entirely sure how well it will perform with different languages and scripts, but it should be no worse than what was there before.
…nent.tsx Co-authored-by: Dennis Kigen <[email protected]>
|
Sorry about the poorly formatted suggestion. Are you getting this too?
|
|
No, I'm not |
|
Do you have the sample config you used to get the screenshot somewhere? |
|
I should've save it, but it was something like: {
"@openmrs/esm-patient-forms-app": {
"formSections": [
{"name": "Section 1", "forms": ["HIV Green Card", "Two-Page Form", "Viral Load Lab Result"]},
{"name": "Section 2", "forms": ["HIV Green Card", "Two-Page Form", "Viral Load Lab Result"]},
{"name": "Section 3", "forms": ["SOAP Note Template", "Test Form 1", "Covid 19", "Test Form", "Laboratory Test Orders"]},
]
}
} |
|
Thanks, it'll be useful when I write up the changelog for the next release. |
|
This reminds me, I think there's an error logged with a config like the one above... It actually works, it's just something about the validation logic. I was going to ticket that up. |
|
Ok. Easy to get lost in the mire. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
Summary
This PR does a few things, but the main one is that it adds the ability to configure "sections" of forms that will display in the form dashboard in places of the normal list of all forms. If this remains unconfigured, nothing is changed, but if configured, it looks like the below UI.
Screenshots
Related Issue
Other
Notably:
Smaller things this does:
orderFormsByNamehas been replaced byorderBywhich can be set to eithername(the default) ormost-recent. Currently, this setting is global to the form list and not configurable per section.There were two components involved in the form, the rather anemic
FormsDashboard, which basically just rendered aFormsList, and theFormsListitself that basically implemented the feature. This refactors things a fair bit to change component responsibility. In particular:FormsTablecomponent which just takes the headers, rows and a couple of helper elements and renders the actual table of forms and search box.FormsListto take a list of forms, an optional section name and simply render an appropriateFormsTable. it also handles searching for the list.FormsDashboardis now responsible for all data loading and then renders aFormsListper section or just aFormsList, depending on configuration.The goal of this refactoring was to move the slowest bits (fetching the list of forms) to the top of the tree (and also to keep
FormsListfrom becoming a madness-inducing series ofif / elseconditions).