-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix when placing flextable footnote to multiple columns and rows #2063
base: main
Are you sure you want to change the base?
Conversation
@ddsjoberg added a test, let me know if it was what you were looking for. Only place where I cannot add a footnote is the stubhead so far |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Daniel for the fix. If It is what you need feel free to merge. From my side, I could not break it :D
Thanks @Melkiades ! Can you take a look into the failing CI/CD? |
…b.com:ddsjoberg/gtsummary into 2062-flextable-multi-row-col-footnote
R/as_flex_table.R
Outdated
\(location_ids, tab_location, row_numbers) { | ||
col_ids <- getElement(location_ids, "column_id") |> unique() | ||
if (tab_location == "body") col_ids <- rep(col_ids, length(row_numbers)) # styler: off | ||
sort(col_ids) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Melkiades , I was doing some final checks, and saw that we were not placing the footnote correctly. I added the sort()
to get it right which changed the footnote placement from this
to
Is there a way to test that the placement of the footnote markers is where we expect in a flextable?
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am on it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!!
@@ -303,22 +303,22 @@ test_that("as_flex_table passes multiple table footnotes correctly", { | |||
modify_table_styling( | |||
columns = stat_1, | |||
rows = (variable %in% "grade") & (row_type == "level"), | |||
footnote = "Cell-level foonotes here." | |||
footnote = "my footnote" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the bug appeared when we were assigning the same footnote to multiple locations. separate footnotes were assigned with separate calls to the flextable footnote function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see! thanks
What changes are proposed in this pull request?
If there is an GitHub issue associated with this pull request, please provide link.
closes #2062
@Melkiades before fully reviewing this PR, can you add a test or two more for different types of footnote placement to ensure they are correctly placed when there is one column and multiple rows, multiple columns and one row, and one mixed-type when the column/rows are contiguous? Thank you!
Reviewer Checklist (if item does not apply, mark is as complete)
pkgdown::build_site()
. Check the R console for errors, and review the rendered website.devtools::test_coverage()
usethis::use_spell_check()
runs with no spelling errors in documentationWhen the branch is ready to be merged into master:
NEWS.md
with the changes from this pull request under the heading "# gtsummary (development version)
". If there is an issue associated with the pull request, reference it in parentheses at the end update (seeNEWS.md
for examples).usethis::use_version(which = "dev")
usethis::use_spell_check()
again