Skip to content

Conversation

@Edwin-Rugoogamu
Copy link

Description

Code review checklist

  • UI/UX backwards compatible: Test it works for the new design (enabled by default). And test it works in the old design, enable can_view_old_navigation permission to see the old design. Test it has appropriate design for RTL languages.
  • Readable: Concise, well named, follows the style guide, documented if necessary.
  • Documented: Configuration and user documentation on cht-docs
  • Tested: Unit and/or e2e where appropriate
  • Internationalised: All user facing text
  • Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

@Edwin-Rugoogamu Edwin-Rugoogamu changed the base branch from master to 10140_previous-month-targets November 29, 2025 17:25
@Edwin-Rugoogamu Edwin-Rugoogamu changed the title 775 add support for having the target subtitles to be a function feat(#775): support function-based target subtitles Nov 29, 2025
{{ target.subtitle_translation_key | translate }}
</p>
<p>
{{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Edwin-Rugoogamu this is not what we discussed, adding two subtitles.
We discussed making target.subtitle_translation_key be a function.

So this would be something like:

target.subtitle_translation_key(reportingPeriodFilter) | translate

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dianabarsan Thankyou for the feedback. let me adjust it .

if (!subtitleKey) {
return '';
}
return reportingPeriod === ReportingPeriod.CURRENT ? 'this_month' : 'all_time';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still doesn't cover what we need to do here. The situation now is that you're not even using the subtitleKey that is configured.

What needs to change here is that subtitleKey will be a function!
So you'll need to do something like:

return typeof subtitleKey === 'function' ? subtitleKey(reportingPeriod) : subtitleKey;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dianabarsan Thankyou for the feedback. We had a meeting with Josh and Team . This elaborated more on what we need to achieve. So it made it clear for me and erased the confusion i was having. So am adjusting and will send a PR for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants