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.
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
Add total row in report tables #179
Add total row in report tables #179
Changes from 9 commits
cfcf025
d3e08a5
1e7e32f
74ec186
a955d61
c42f1e6
62e79e6
19568cb
d4a2f00
8655365
25e72a8
bd43c6f
880279d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Can you explicit why this change is needed?
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.
The return value of
valueGetter
is used for sorting and it's being passed in thevalueFormatter
orrenderCell
. With this change:params.value
in therenderCell
without the need to duplicate the logic to construct the value.From docs:
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.
We can use the same approach here to calculate
formattedList
once in thevalueGetter
dmp/apps/frontend/utils/tableFormatting.tsx
Lines 121 to 139 in 74ec186
I skipped this as I thought it was out of scope and required more testing, but we can make that change since we are working on this area
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.
Ok, worth exploring. Two things come to mind though:
Ideally, for sorting, we might actually want to order by number of villages instead of the alphabetical order of the first village (which doesn't really mean anything). Noting that sorting doesn't really work for the village column at the moment, esp. in "Commune" reports.
We still need to display the tooltip