Open-Systems-Pharmacology / TLF-Library

TLF Library implementation in R
https://www.open-systems-pharmacology.org/TLF-Library/
Other
9 stars 6 forks source link

473 improve handling of loooooong plot and grid titles #474

Closed Felixmil closed 1 year ago

Felixmil commented 1 year ago

We can now use insanely long strings for labels and legend without overflow outside of the plot and plotgrids

image

image

Warning A lot of snapshots will break on ospsuiteR

Felixmil commented 1 year ago

For some reason, the factor order for groups is different in local than appveyor, causing the build to fail.

Appveyor's snapshot: image

Local's snapshot: image

@pchelle any idea why it does that ?

pchelle commented 1 year ago
* Use ggtext and stringr as new dependency (both validated)

I did not know that ggtext was already validated. If so, that means we could include markdown content within plots ! :) But we would need to spread all this to the other texts/labels (e.g. axis.text, axis.title, etc.) correct ?

pchelle commented 1 year ago

A lot of snapshots will break on ospsuiteR

Potentially also on Reporting Engine. Not sure if this will crash or not some plots or change their look. I also don't know how the translation of the fonts from showtext will perform.

pchelle commented 1 year ago

@pchelle any idea why it does that ?

One of my guesses is that depending on one or some of the versions of your packages, the legend ordering may be different as it already happened for ggplot2 version 3.3.0 (see below)

https://github.com/Open-Systems-Pharmacology/TLF-Library/blob/daea5899dbcb989a6c9f7be15d4b1a319a18f635/R/utilities-legend.R#L166

A smart R6 object might be a solution to better handle legends through guides and scales.

Felixmil commented 1 year ago
* Use ggtext and stringr as new dependency (both validated)

I did not know that ggtext was already validated. If so, that means we could include markdown content within plots ! :) But we would need to spread all this to the other texts/labels (e.g. axis.text, axis.title, etc.) correct ?

Yes !

This is already spread to: title, subtitle, caption and axis titles. But not for axis text (because rotations are only by increment of 90° in ggtext and user may want to apply 30/45°)

Felixmil commented 1 year ago

A lot of snapshots will break on ospsuiteR

Potentially also on Reporting Engine. Not sure if this will crash or not some plots or change their look. I also don't know how the translation of the fonts from showtext will perform.

My first test on OSPSuite R produced same plots but with slighly increase margins. The font seemed identical.

Felixmil commented 1 year ago

@pchelle This is ready for review

pchelle commented 1 year ago

@pchelle This is ready for review

Except for the maxWidth and charactersWidth comments, all good.

Felixmil commented 1 year ago

Except for the maxWidth and charactersWidth comments, all good.

Which comments ?

pchelle commented 1 year ago

Except for the maxWidth and charactersWidth comments, all good.

Which comments ?

Felixmil commented 1 year ago

I am sorry, I seem to not to be able to access these sections 😅

PavelBal commented 1 year ago

@pchelle Forgot to submit the review? :D

pchelle commented 1 year ago

@pchelle Forgot to submit the review? :D

Indeed ! 😅