-
Notifications
You must be signed in to change notification settings - Fork 213
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
Add support for rownames_to_stub
+ opt_interactive()
#1706
Conversation
This is great! I wouldn't worry about the width. There's a default width here that can be set for all widths in |
Also, I wouldn't worry about the adding of a test here. |
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.
LGTM!
I've reviewed and approved. If there is nothing more to change here, I'll merge once all tests pass. |
Thanks for the review. All good on my side, but let's wait for test-coverage to pass! I have tried to address #1706, but realized it was not as straight forward as I had imagined. I may take another look later on. |
Thanks for taking on these bugs! If you're interested in working on #1510, it's along the same lines (but not interactive) and also a pretty serious bug. I suspect the issue there is that we are not supplying enough column width values and columns get dropped because of that. |
Hi @rich-iannone. I may try again later, but here is the code I used in case it is useful to you tb <- mtcars %>%
dplyr::select(mpg, cyl, vs) %>%
head() %>%
gt(groupname_col = "vs", row_group_as_column = T)
one_width <- tb %>%
cols_width(
mpg ~ px(100),
)
all_widths <- tb %>%
cols_width(
mpg ~ px(100),
cyl ~ px(100)
)
#' Somehow the only thing inside the gt object that is not different
waldo::compare(
one_width,
all_widths
)
waldo::compare(one_width %>% render_as_html(), all_widths %>% render_as_html(), x_arg = "working", y_arg = "bug", max_diffs = Inf, )
one_width %>% render_as_html() %>%
.[1] %>%
stringr::str_split_1(pattern = "\n") %>%
.[1:8]
all_widths %>% render_as_html() %>%
.[1] %>%
stringr::str_split_1(pattern = "\n") %>%
.[1:8]
#' # As a comparison, with `row_group_as_column = FALSE`
#'
tb2 <- mtcars %>%
dplyr::select(mpg, cyl, vs) %>%
head() %>%
gt(groupname_col = "vs", row_group_as_column = F)
one_width2 <- tb2 %>%
cols_width(
mpg ~ px(100),
)
all_widths2 <- tb2 %>%
cols_width(
mpg ~ px(100),
cyl ~ px(100)
)
#' Somehow the only thing inside the gt object that is not different
# same as difference between one_width and all_widths
waldo::compare(one_width2 %>% render_as_html(), all_widths2 %>% render_as_html(), x_arg = "one_width", y_arg = "all_widths")
# same as the comparison above, so the issue is not in how gt sets col widths, the issue lies in the render html part.
waldo::compare(
one_width2,
all_widths2
)
waldo::compare(one_width2 %>% render_as_html(), all_widths2 %>% render_as_html(), x_arg = "one_width", y_arg = "all_widths")
# not very useful
waldo::compare(
all_widths2 %>% render_as_html(),
all_widths %>% render_as_html(),
x_arg = "working",
y_arg = "bug",
max_diffs = Inf
)
# not very useful.
waldo::compare(
one_width2 %>% render_as_html(),
one_width %>% render_as_html(),
x_arg = "working",
y_arg = "working row_group_as_col"
)
Next steps for checking,
The issue may be caught more by a general html validator as locally, the rendered code doesn't tell us much... Do you have tip on which helper function's output I should look at to really find out what is going wrong? |
Thanks for looking into this one. For that issue, I think the problem is within |
rowname_to_stub
+ opt_interactive()
rownames_to_stub
+ opt_interactive()
fixes #1702
Do you think it is worth adding a new test?Maybe the width is too wide? I don't know how to tweak it.This PR
Before