Skip to content

feat: add dynamic control for overall events/patients in hlt split (#1496)#1540

Open
ygma-bitc wants to merge 3 commits into
insightsengineering:mainfrom
ygma-bitc:fix-1496-overall-events
Open

feat: add dynamic control for overall events/patients in hlt split (#1496)#1540
ygma-bitc wants to merge 3 commits into
insightsengineering:mainfrom
ygma-bitc:fix-1496-overall-events

Conversation

@ygma-bitc

Copy link
Copy Markdown

Description

This PR introduces dynamic UI controls to toggle the overall event and patient summary rows when both HLT and LLT are selected in the tm_t_events module.

Fixes

Resolves #1496

Changes Made

  • Added incl_num_patients_hlt and incl_num_events_hlt checkbox inputs to the UI.
  • Updated template_events to dynamically render tern::summarize_num_patients based on UI inputs.
  • Implemented scorefun_hlt_no_sum to preserve correct HLT sorting behavior when summary rows are disabled.
  • Updated corresponding shinytest2 E2E tests to cover the new UI toggle behaviors.

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

✅ All contributors have signed the CLA
Posted by the CLA Assistant Lite bot.

@ygma-bitc

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@ygma-bitc
ygma-bitc force-pushed the fix-1496-overall-events branch from c56e932 to 1f7b7ce Compare June 1, 2026 14:21
@osenan osenan self-assigned this Jun 17, 2026
@llrs-roche

Copy link
Copy Markdown
Contributor

Thanks for the patch but this PR adds 100 lines of code without even agreeing this is something we should modify on the module. In addition it adds more teal.transform inputs while we are moving to use teal.picks...
Could you explain the importance of this feature?

@ygma-bitc

Copy link
Copy Markdown
Author

Hi @llrs-roche,

Thanks for the review and the heads-up on the teal.picks roadmap!

We ran into this limitation in our own project and noticed that @Xiaolin-S raised the same issue in #1496, so we just wanted to share our local fix with the community.

Since the team is planning to transition the module to teal.picks, I completely understand if this implementation doesn't align with the new design. I'm very much looking forward to seeing this feature in future updates instead!

Feel free to close this PR if you prefer to handle it natively during the migration.

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.

[Question]: <“Overall total number of events” in tm_t_events module>

3 participants