Skip to content

Feature/jgqo373 watermark decorator#23

Merged
kpagacz merged 16 commits intomainfrom
feature/jgqo373_watermark_decorator
Mar 3, 2026
Merged

Feature/jgqo373 watermark decorator#23
kpagacz merged 16 commits intomainfrom
feature/jgqo373_watermark_decorator

Conversation

@NadiaAbraham
Copy link
Collaborator

New watermark decorator for plots. Currently we are not supporting rtables

@NadiaAbraham NadiaAbraham requested a review from kpagacz February 6, 2026 14:12
output_name <- gridify::gridify(
object = cowplot::as_grob(output_name),
layout = gridify_layout
) %>%
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not use %>% in the decorators if we don't have to do it. There's |> which is a sufficient replacement here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

object = cowplot::as_grob(output_name),
layout = gridify_layout
) %>%
set_cell("watermark", watermark_text)
Copy link
Collaborator

Choose a reason for hiding this comment

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

set_cell is from rtables?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

set_cell is from gridify...added package reference

res <- within(
res,
{
watermark_text <- txtWatermark
Copy link
Collaborator

Choose a reason for hiding this comment

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

I fail to see the point of this line

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed redundant code

Comment on lines +8 to +9
#' @param watermark_text (`character(1)`) text to display for the watermark
#' @param font_size (`character(1)`) font size for the watermark text
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#' @param watermark_text (`character(1)`) text to display for the watermark
#' @param font_size (`character(1)`) font size for the watermark text
#' @param watermark_text (`character(1)`) text to display for the watermark.
#' @param font_size (`character(1)`) font size for the watermark text.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

Comment on lines +13 to +14
#' @details The module creates a UI with textInput for specifying watermark text and
#' font size.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#' @details The module creates a UI with textInput for specifying watermark text and
#' font size.
#' @details The module creates a UI with `textInput` for specifying watermark text and
#' font size.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

#'
#' @details The module creates a UI with textInput for specifying watermark text and
#' font size.
#' the entered watermark text is displayed with a default gridify layout.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#' the entered watermark text is displayed with a default gridify layout.
#' the entered watermark text is displayed with a default `gridify` layout.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In case you are wondering why it matters - part of the CI is spell checking the package. Anything inside `` is not spell checked, anything outside is. gridify is not a real word, so it will be flagged by our spell check

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure Konrad...made the update

#' font size.
#' the entered watermark text is displayed with a default gridify layout.
#'
#' @import cowplot gridify rtables.officer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is rtables.officer needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was needed for the tables. Will remove as we are not enabling this decorator for tables now. Thanks for pointing it out

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

badge

Code Coverage Summary

Filename                                     Stmts    Miss  Cover    Missing
-----------------------------------------  -------  ------  -------  ---------
R/BlockConditions.R                             14      14  0.00%    23-54
R/create_rel_risk_transformator.R               43      43  0.00%    36-84
R/forest_plot_decorator.R                      164     164  0.00%    97-373
R/ggplot_decorator.R                           244     244  0.00%    84-355
R/merge_levels_transformator.R                 146     146  0.00%    11-240
R/or_filtering_transformator_view_model.R       27      27  0.00%    26-64
R/or_filtering_transformator.R                 254     254  0.00%    69-369
R/patchwork_plot_decorator.R                    46      46  0.00%    18-68
R/r_access_utilities.R                          46      46  0.00%    32-152
R/title_footer_decorator.R                     130     130  0.00%    103-241
R/watermark_decorator.R                         61      61  0.00%    23-87
TOTAL                                         1175    1175  0.00%

Diff against main

Filename                          Stmts    Miss  Cover
------------------------------  -------  ------  --------
R/merge_levels_transformator.R       +6      +6  +100.00%
R/watermark_decorator.R             +61     +61  +100.00%
TOTAL                               +67     +67  +100.00%

Results for commit: 4f0813e

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@kpagacz kpagacz self-requested a review March 3, 2026 13:27
@kpagacz kpagacz merged commit ea765aa into main Mar 3, 2026
25 checks passed
@kpagacz kpagacz deleted the feature/jgqo373_watermark_decorator branch March 3, 2026 20:19
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