feat(#10240): hide target counts past goal#10772
feat(#10240): hide target counts past goal#10772cliftonmcintosh wants to merge 15 commits intomedic:masterfrom
Conversation
|
Who should I ask to review this? |
Replace absolute positioning of goal label with flex column layout to prevent overlap with large count numbers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ssion Add can_hide_target_count_past_goal permission to control whether the count number is hidden for targets where pass >= goal. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ion and goal combinations Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…default config Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bb658e7 to
95e1d0f
Compare
| display: flex; | ||
| flex-direction: column; | ||
| align-items: center; |
There was a problem hiding this comment.
Centering the goal seems cleaner than trying to pad it in an absolute position in the top right corner. See my question about this in the PR description.
|
She's offline already, but @dianabarsan should be able to pick this up next week - thanks for reaching out! FYI we're working with a bunch of new community members who are submitting a big collection of PRs - might be a hot minute but we'll get to this! |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dianabarsan
left a comment
There was a problem hiding this comment.
Thanks for the quick work. I think the central alignment is the best (but I'm not a UX expert).
As for hiding the target count when it exceeds the goal, I believe we weren't trying to hide the actual number (like in your screenshots), rather just show the goal number instead of the actual target number. It's possible using a permission is not the best approach, maybe only some targets need their goals hidden?
I believe the context for hiding the exceeding number past the goal was in the cases where the target counts provided remuneration, in some cases once the "ceiling" (goal) was reached, not additional remuneration was offered.
for example, the chw would get paid:
- $50 for a score of 500,
- $100 for a target score of 1000 (the goal / ceiling)
- $100 for a target score of 1500
Users would sometimes expect to be paid $200 if their target showed 2000. So then, these targets specifically would not show the actual number if it exceeded the goal.
For other targets, for example for pregnancy or delivery targets, i see absolutely no reason to hide the actual target number.
| this.authService.has(HIDE_COUNT_PAST_GOAL_PERMISSION).then(has => { | ||
| this.hideCountWhenGoalMet = has; | ||
| }); |
There was a problem hiding this comment.
can this be simplified as ?
| this.authService.has(HIDE_COUNT_PAST_GOAL_PERMISSION).then(has => { | |
| this.hideCountWhenGoalMet = has; | |
| }); | |
| this.hideCountWhenGoalMet = await this.authService.has(HIDE_COUNT_PAST_GOAL_PERMISSION); |
There was a problem hiding this comment.
Applied. I had to define ngOnInit as async, so I made a commit locally and pushed it
Ahhh. That makes sense. I should be able to adjust it so that when the flag is set and the goal is exceeded, we show the goal instead of the target count.
Is there a way to distinguish which targets should not be hidden?
Given the possiblity that some targets might not be hidden (like pregnancy or delivery targets) while some might be, what about an alternative approach like this: Add a new field to the target data structure that allows people to say the target can't exceed the goal. Once the goal is reached, either the app stops incrementing the data or, if it does increment, we use that new field to decide to limit the target count in the UI to the goal amount. |
Co-Authored-By: Diana Barsan <35681649+dianabarsan@users.noreply.github.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
I believe adding a field to the target configuration is the way to go. There is no such field at the moment (hence the feature request :D :D )
I think this will have more implications. I think we should just manifest this setting in the UI and have the target document still register the actual number. |
@dianabarsan Thanks. Based on the feedback so far, can I state what I think the goals are?
Does that capture what we're thinking? |
|
@cliftonmcintosh nailed it!! |
…al is set Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… goals Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…et aggregation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
webapp/src/css/targets.less
Outdated
| justify-content: flex-end; | ||
| text-align: center; | ||
| min-height: 74px; |
There was a problem hiding this comment.
The change for justify-content and min-height aligns the count number display across count widgets that have a goal and those that do not.
Previously, the number position differed between the two variants, causing visual inconsistency
when both types appeared on the same Targets tab. Fixed by adding a minimum height to the number container and using content justification to keep the number vertically centered regardless of whether a goal is present.
There was a problem hiding this comment.
Yea, I don't like the big number offset either. I'm wondering whether the best visual solution is to move the goal underneath the number and conditionally display it. Then we won't have all this extra padding.
There was a problem hiding this comment.
I'm wondering whether the best visual solution is to move the goal underneath the number and conditionally display it. Then we won't have all this extra padding.
I can work up that as an option and provide a screenshot. Or if you are confident that is a change to make, I can go ahead and do it. Let me know, please.
|
Will the test at |
|
As I understand it, if we want an e2e test for the new field, we will depend on the If so, I think that means we would pause this until a new version of cht-conf is published and can be brought in as a dependency in cht-core. |
|
I didn't end up having space for this today. I'll check on it tomorrow. |
No, you can safely ignore those. They haven't been introduced in our CI suite and not run regularly. |
Not necessarily. You can install a branch cht-conf version instead of the latest release in this branch, and then the property will be available to use. We would, indeed, need to publish the cht-conf package before merging this PR, as reverting to the latest released cht-conf will be required. |
dianabarsan
left a comment
There was a problem hiding this comment.
So far excellent work, as usual!
I added one question about alternative styling and replied to the e2e questions.
webapp/src/css/targets.less
Outdated
| justify-content: flex-end; | ||
| text-align: center; | ||
| min-height: 74px; |
There was a problem hiding this comment.
Yea, I don't like the big number offset either. I'm wondering whether the best visual solution is to move the goal underneath the number and conditionally display it. Then we won't have all this extra padding.
| } | ||
|
|
||
| getDisplayCount(target: any): number { | ||
| if (target.limit_count_to_goal && target.goal >= 0 && target.value?.pass >= target.goal) { |
There was a problem hiding this comment.
Awww, negative goals are out? is the goal positive check necessary?
There was a problem hiding this comment.
ah I see, it's in case someone creates a target without a goal and then sets limit_count_to_goal to true?
There was a problem hiding this comment.
it's in case someone creates a target without a goal and then sets limit_count_to_goal to true
Yes!
If it makes it easier to review, I can go ahead and do this and then we can revert to the latest after the cht-conf change is merged. Please let me know if you prefer that or prefer to wait until the cht-conf changes are merged. |
We have done this numerous times, I prefer it and suggest it. No reason to block on the other work. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…across target cards Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
I've added e2e tests with the cht-conf dependency set to my branch. |
| "chai-shallow-deep-equal": "^1.4.6", | ||
| "chai-things": "^0.2.0", | ||
| "chokidar": "^4.0.3", | ||
| "cht-conf": "^6.0.2", |
There was a problem hiding this comment.
Reminder that this needs to be changed before merging.
There was a problem hiding this comment.
The changes in this file are because of the temporary change to the cht-conf dependency. They should be reverted when we update that dependency again.



Description
Outdated goals
This original approach has been superseded. See the Updated goals immediately below. For more context, please read the discussion.
Layout overlap: When a count number is large, it overlapped with the "Goal X" label. Fixed by converting the
.countblock to a flex column layout so the goal label and count number stack vertically instead of colliding.Hide count past goal: Adds a new permission
can_hide_target_count_past_goal. When a user has this permission, the count number is hidden on any target wherepass >= goal. The goal label remains visible. This behavior is off by default (empty roles array in the default config).Updated goals
Closes #10240
Companion PRs
CHT-CONF: medic/cht-conf#800
CHT-DOCS: medic/cht-docs#2176
Questions
Layout overlap fix — approach feedback
I fixed the overlap by replacing the absolute-positioned goal label with a stacked flex layout (
.countbecomesdisplay: flex; flex-direction: column; align-items: center, with the goal label above the count number, both centered). An alternative I considered was keeping the goal label in the top-right corner and adding right padding to.numberwhen.has-goalis present (e.g.,.has-goal .count .number { padding-right: 80px; }), which would be a smaller change but would offset the count number from true center. The padding approach would keep the count number visually off-center — it would be centered over the remaining space after reserving room for the goal label, not centered over the full card width. So on a wide card with a goal label, the big number would appear shifted left rather than truly centered. This approach is also fragile — it's a magic number that works for typical cases but can't guarantee no overlap for arbitrarily large numbers. Does the centered stacked layout work for you, or would you prefer the top-right approach?Here is an example of the centering approach
Here is an example of the padding approach
What to hide
First off, if we center the goal, then do we need hiding? I've included it, but I want to raise the possibility that it might not be needed if we center the goal. I'm not sure what the use cases are, so I defer to you.
In one of @dianabarsan's comments, she said we should be
I chose to not display the target count because that was what I understood the suggestion to be. The example feature flag (
can_hide_target_count_past_goal) mentioned in the issue points to that as the approach that was wanted, but it feels to me like having an empty target makes it look like the data is missing or the person has not met the goal.What is preferred? Hiding the target? Hiding the goal? Something else? Here are some screenshots with different options. I think Option 2 works best.
I also wonder if maybe this isn't needed if we center both the goal and the target.
Option 1: Currently implemented -- hide only the target when goal is met or exceeded
Option 2: Hide only the goal when goal is met or exceeded
Option 3: Hide both the target and the goal when goal is met or exceeded
Option 4: Include a visual element showing the goal is met
AI Disclosure
I used Claude Code to help navigate the codebase, plan the approach, and write tests. I made all design decisions, reviewed all code, and verified the solution manually in the browser. I attributed Claude as a co-author on all commits that included the use of Claude Code.
If you would like to see them, I can provide a copy of the directions I gave Claude Code when starting the planning.
Code review checklist
can_view_old_navigationpermission to see the old design. Test it has appropriate design for RTL languages.Visual alignment fix for count widgets
I adjusted the alignment of the target tiles to aligns the count number display across count widgets that have a goal and those that do not.
Previously, the number position differed between the two variants, causing visual inconsistency
when both types appeared on the same Targets tab. Fixed by adding a minimum height to the number
container and using content justification to keep the number vertically centered regardless of
whether a goal is present.
Testing done
Unit tests
14 unit tests cover the
AnalyticsTargetsComponentinwebapp/tests/karma/ts/modules/analytics/analytics-targets.component.spec.ts.The following cases are specifically tested for the
limit_count_to_goalbehavior:limit_count_to_goal: true,pass > goal→ displaysgoalas the count numberlimit_count_to_goal: true,pass === goal(boundary case) → displaysgoallimit_count_to_goal: true,pass < goal→ displays actualpassvaluelimit_count_to_goalabsent → displays actualpassvalue even when goal is exceededlimit_count_to_goal: true,goal: -1(no goal) → displays actualpassvalueAll 14 tests pass.
Browser verification with dummy data
A dummy target (
Earned Points,goal: 20000) was temporarily added to the componentto visually verify behavior in the browser under the following conditions:
limit_count_to_goalpassgoaltruetruetrueScreenshots from testing
Screenshot: limit when target exceeds goal
Screenshot: no limit enforced when target exceeds goal
Screenshot: limit when target less than goal
Screenshot: limit when target equals goal
Screen recording: mobile-device-sized view
mobile-device-view-when-target-exceeds-goal.mov
End-to-end verification with real config data
The
active-pregnanciestarget inconfig/default/targets.jswas temporarily modifiedto
goal: 20, limit_count_to_goal: trueand uploaded to the running local instance usingthe local
cht-confbuild (which includes the companion schema and parse-targets changes).After logging in as a CHW user with 33 active pregnancies, the target card correctly
displayed 20 (the goal) instead of 33 (the actual count), confirming the full data flow:
targets.js→cht-conf compile→ CouchDB →rules-engine→target-state.js→ component → UIScreenshot: testing limit in view with active pregnancies
License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.