insightsengineering / teal.reporter

Create and preview reports with Shiny modules
https://insightsengineering.github.io/teal.reporter/
Other
8 stars 9 forks source link

[Question]: Placement of `card_template()` function. #226

Closed kartikeyakirar closed 9 months ago

kartikeyakirar commented 9 months ago

What is your question?

To include card labels, the add_card_button_srv function allows you to specify the card_fun along with a label parameter for customizing the card's title and content. This feature enables you to utilize the label shiny input and incorporate it into your card_fun to set the title.

When it comes to teal modules, each module has its own structure for the card_fun, but there are common elements shared across all card_function that can be standardized within a card_templatefunction. You can find an example of this in the following code:

Teal Reporter - card_template function

This card_template function can be further encapsulated to suit specific requirements, as demonstrated here:

Teal Goshawk - customized card_template

The challenge lies in deciding where to place the card_template function, as keeping it within teal.reporter adds teal as a dependent package. Here are a few suggestions:

Option 1: Move the card_template()function to teal as an exported function. Instead of teal.reporter.

Option 2: Create a separate card_template for each module (e.g., tmc, tmg, tmh) to ensure module-specific customization.

Option 3: Consider adding teal as a dependency to teal.reporter to retain the card_template function.

kartikeyakirar commented 9 months ago

@insightsengineering/nest-core-dev Please share your thoughts or questions regarding

m7pr commented 9 months ago

Why do we need teal package for that?

On Wed, Oct 11, 2023, 20:23 kartikeya kirar @.***> wrote:

Assigned #226 https://github.com/insightsengineering/teal.reporter/issues/226 to @m7pr https://github.com/m7pr.

— Reply to this email directly, view it on GitHub https://github.com/insightsengineering/teal.reporter/issues/226#event-10622422038, or unsubscribe https://github.com/notifications/unsubscribe-auth/A74AIELLK772UU47SERTWZ3X63PY3ANCNFSM6AAAAAA54NCFXQ . You are receiving this because you were assigned.Message ID: <insightsengineering/teal.reporter/issue/226/issue_event/10622422038@ github.com>

kartikeyakirar commented 9 months ago

Because the card_template function relies on the teal package, it utilizes the TealReportCardR6 class, which has been relocated to the tealpackage.

m7pr commented 9 months ago

Alrighty, so it's rather clear that you need to include it (card_template) in teal and we don't need a separate discussion about it. No need for extra dependencies in teal.reporter then

On Thu, Oct 12, 2023, 05:45 kartikeya kirar @.***> wrote:

Because the card_template function relies on the teal package, it utilizes the TealReportCard R6 class, which has been relocated to the teal package.

— Reply to this email directly, view it on GitHub https://github.com/insightsengineering/teal.reporter/issues/226#issuecomment-1758866481, or unsubscribe https://github.com/notifications/unsubscribe-auth/A74AIEPSJB75YWGPSQHJUQ3X65RWZANCNFSM6AAAAAA54NCFXQ . You are receiving this because you were mentioned.Message ID: @.***>

m7pr commented 9 months ago

Oh yeah, and as we just spoke, teal.reporter can not import teal, since teal is already importing teal.reporter 📦

kartikeyakirar commented 9 months ago

I've moved the template function to the teal repository. You can find the corresponding pull request at this link: https://github.com/insightsengineering/teal/pull/933.