Skip to content

Conversation

@olivroy
Copy link
Collaborator

@olivroy olivroy commented Jul 15, 2024

Summary

@teunbrand I'd love to have your views here in general.

  • Does this seem reasonable?
  • Should we use vdiffr to capture the tables as a useful visual reference?
  • I am not too sure of what I did here. My testing is mostly through trial and error.
  • Any concerns with unicode?

Related GitHub Issues and PRs

@rich-iannone I am not too sure if I am duplicating things here. Not clear to me how context = "plain" vs context ="grid" should act.

It is just that grid renders unicode formats, since R 4.2 I think?

Do you think we may add some conditions on R version for some grid features? i.e. if context = "grid" and R version < 4.2, use plain text rendering?

Examples

This PR resolves the following to look the same in html and grid

# this works with only a few warnings!
info_currencies() |> plot()
gt_tbl <- exibble |> 
  gt() |> 
  cols_label(
    num = md("num<br>x")
  )

plot(gt_tbl)

Previously, we would see <br>

@olivroy olivroy changed the title Improvements to grid context Improvements to grid context as_gtable() + plot.gt_tbl() Jul 15, 2024
@rich-iannone
Copy link
Member

@olivroy

I don't think we should limit grid table output to those using R 4.2 or later in practice (unless there are errors). The better thing to do might be to include some notes about this in the new FAQ section (great addition btw!). The topic could generally outline the capabilities/problems of the different table output types.

The context of "grid" seems good to me instead of using "plain" (and we ought to better define what that really is, my estimate right now is ASCII formatting outputs). We should have another context like "utf8" (or similar) for formatting outputs that can include characters outside the ASCII range.

One more thing, we ought to think of visual tests or just additional Quarto websites for grid-table outputs (and also for LaTeX, that's probably more important). I'll try working on this a bit (the Quarto website for grid-table examples) and I think it just involves adding a plot statement to certain example statements that produce a gt table.

Copy link
Member

@rich-iannone rich-iannone left a comment

Choose a reason for hiding this comment

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

LGTM!

@teunbrand
Copy link
Contributor

Does this seem reasonable?

It doesn't look unreasonable, but I don't really know what markdown::mark() or commonmark::markdown_text() are doing, so feel free to dismiss.

Should we use vdiffr to capture the tables as a useful visual reference?

You can, but it is an extra suggest dependency that I'm not sure pays off here. For testing purposes the layout object created in the line below should have most of the relevant components (i.e. labels/layout) to test:

gt/R/export.R

Line 1194 in 34fd17f

layout <-

Any concerns with unicode?

No grid should render unicode just fine even before R 4.2.0 I think so this shouldn't be cause for worry. Potentially it could be the case that a unicode glyph is not in the font, so it might render as missing characters in devices that don't have font fallbacks. But that's just a limitation, so there is nothing that {gt} can do here.

@olivroy
Copy link
Collaborator Author

olivroy commented Jul 15, 2024

Maybe I can provide an explanation here. It feels a bit magical, but since the currencies are stored in html strings, I discovered that markdown::mark(gt:::currencies$symbol, format = "text") transformed the html codes to their utf-8 equivalent. (This allows us showing the currency symbol instead of their unescaped equivalent) GBP for British pounds for example.

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.

3 participants