search

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

Add customizable margin around text #484

Closed Felixmil closed 1 year ago

Felixmil commented 1 year ago

Texts come with good default margins (defined in minimalTheme) but they can also be customized in Label objects

before after
image image
image image
Felixmil commented 1 year ago

@PavelBal I have a similar issue than yesterday. On Appveyor, difference is detected in svg code but the plots looks the same. On my computer no diff are detected during test run. Could you please pull this branch and run test / push snapshots if there are somehow different ? You can run usethis::pr_fetch(484) to get it.

PavelBal commented 1 year ago

@Felixmil I have re-created example reports with this version, and for the rare use case where the title has a linebreak the new version actually looks worse:

https://github.com/esqLABS/esqlabsR.reports/pull/13/files#diff-9f54c02fe5979e9de1bebc7af953539c9bb457a076efdf3c327fda1488902631

Other examples, like this, look better:

https://github.com/esqLABS/esqlabsR.reports/pull/13/files#diff-bf9dfebbcb1ffe0fc6b6d9642b2dd231bb6a0215843a9f46028ef6a0c4b38e18

Any chance to either a) make margins dynamic based on the heights of the elements (e.g. the title), or b) set defaults that at least titles with two lines fit in?

Felixmil commented 1 year ago

I will try to find better default values as it is not possible to retrieve plot element sizes. And also, most of other plots with linebreaks still look fine: https://github.com/esqLABS/esqlabsR.reports/pull/13/files#diff-960fdfdacdbd3571e10c5ceaf37507efeb1a49d7d57695626e55266063e22144

However for special cases like this, I would use the ability to customize margins around text (that is implemented in tlf in this branch but still needs to be implemented in osps afterward) or reduce legend/title text size

Felixmil commented 1 year ago

@Yuri05 Ugrading the R_VERSION to the latest available (e.g 4.3.1 vs 4.1 defined before) for AppVeyor configuration solved the difference in snapshots. This is because some package implicated in the .svg generation changed between these two R versions.

What is the right thing to do here, keep the updated R_VERSION in Appveyor or setting my development environment back to 4.1 ?

PavelBal commented 1 year ago

setting my development environment back to 4.1 ?

Would be very uncomfortable..

Yuri05 commented 1 year ago

@Yuri05 Ugrading the R_VERSION to the latest available (e.g 4.3.1 vs 4.1 defined before) for AppVeyor configuration solved the difference in snapshots. This is because some package implicated in the .svg generation changed between these two R versions.

What is the right thing to do here, keep the updated R_VERSION in Appveyor or setting my development environment back to 4.1 ?

update appveyor version to "4.3.1" :) (same for ospsuite-R and ospsuite.utils if not done yet)

Felixmil commented 1 year ago

update appveyor version to "4.3.1" :) (same for ospsuite-R and ospsuite.utils if not done yet)

But the R version in the DESCRIPTION file must stay the same ?

Yuri05 commented 1 year ago

But the R version in the DESCRIPTION file must stay the same ?

I would say: yes (or are we aware of any reasons why tlf should not work with R 3.6?)

Felixmil commented 1 year ago

I would say: yes (or are we aware of any reasons why tlf should not work with R 3.6?)

It will work and is ok as long as we don't expect outputs generated across R versions to be identical.

Yuri05 commented 1 year ago

It will work and is ok as long as we don't expect outputs generated across R versions to be identical.

that's fine; let's keep the version AS IS than :)