(fix)O3-3046: Allow sorting vitals, biometrics and conditions rows based on different sort functions #1779
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
vasharma05
requested review from
denniskigen,
ibacher and
hadijahkyampeire
and removed request for
denniskigen and
ibacher
|
Size Change: -177 kB (-2%) Total Size: 11.1 MB
ℹ️ View Unchanged
|
ibacher
approved these changes
Apr 8, 2024
denniskigen
approved these changes
Apr 8, 2024
| sortFunc: (valueA: ConditionTableRow, valueB: ConditionTableRow) => number; | ||
| } | ||
|
|
||
| export function useConditionsSorting(tableHeaders: Array<ConditionTableHeader>, tableRows: Array<ConditionTableRow>) { |
There was a problem hiding this comment.
Would something like useSortedConditionTableRows describe this hook better?
| { | ||
| key: 'muac', | ||
| key: 'dateRender', |
There was a problem hiding this comment.
What's the rationale behind the Render suffix?
There was a problem hiding this comment.
There are 2 kinds of data for a column (say date column):
date, which is the ISO stringdateRender, which is the formatted version of the pt.1 date.
Now, if we see, the user/ datatable sees the dateRender on the screen, but we want to sort the column based on the date value, hence I added the "Render" suffix to differentiate and keep the original value intact.
The sortFunc of the header will be using the date for comparison, and the DataTable will be using dateRender for rendering data in the column.
| { | ||
| key: 'temperature', | ||
| key: 'dateRender', |
There was a problem hiding this comment.
Is the idea to repackage this logic as a custom hook, similar to what you did with conditions above?
There was a problem hiding this comment.
Yes @denniskigen, I'll move this hook inside the framework such that all the data tables can use this similar implementation.
The new need from the developer will be to add the sortFunc for every header in the tableHeaders.
If you have a better idea than this, I'm all open to hear it.
Thansk!
|
The failing vitals and biometrics tests are likely due to a change in the default sort order behaviour. |
|
Fixed the failing tests @denniskigen |
3 tasks
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 adds custom sorting functions for every column in the vitals, biometrics and conditions table.
Screenshots
Screen.Recording.2024-04-08.at.15.50.01.mov
Related Issue
https://issues.openmrs.org/browse/O3-3046
Other
None