coatless / quarto-webr

Community developed Quarto Extension to Embed webR for HTML Documents, RevealJS, Websites, Blogs, and Books.
https://quarto-webr.thecoatlessprofessor.com/
394 stars 19 forks source link

[Feature]: Allow user defined code cell option defaults and user defined editor font size #172

Closed ute closed 7 months ago

ute commented 8 months ago

Feature Description

Hi James, you have created fantastic new features in the last months, loveit, thank you so much!

I found the editor font a little small when creating revealjs slides. It would be cool to be able to set that font size as a user. It would also be nice to set this globally. I've made a fork with a suggestion for that, it also contains an example qmd in the root. I am not sure if this automatically became a pull request - from my side it looks as if yes.

Best, Ute

coatless commented 8 months ago

@ute both contributions would make great additions! I appreciate the PR! Thank you.

Regarding the editor-font-size and global options, what would you think about incorporating additional structure, e.g. font-size under editor?

webr:
   global-options:
       editor:
          fontsize: '17.5rt'
          word-wrap: 'true'
          theme: 'light'

Also, I suppose one question arises related to built-in theme style, e.g. fontsize, should we try to respect these Quarto values if set or does that not make sense?

https://quarto.org/docs/output-formats/html-themes.html#basic-options

(As far as I know, the only portion that fontsize does not impact is the text in the editor itself.)

ute commented 8 months ago

Regarding the editor-font-size and global options, what would you think about incorporating additional structure, e.g. font-size under editor?

webr:
   global-options:
       editor:
          fontsize: '17.5rt'
          word-wrap: 'true'
          theme: 'light'

Great! very nice and clean!

Also, I suppose one question arises related to built-in theme style, e.g. fontsize, should we try to respect these Quarto values if set or does that not make sense?

https://quarto.org/docs/output-formats/html-themes.html#basic-options

(As far as I know, the only portion that fontsize does not impact is the text in the editor itself.)

Absolutely! I could not figure out how to map font size in the editor text to em or rem - it becomes tiny if set to 1em, and the reason seems to be that Monaco just ignores any length unit given and just takes the number as number of pixels, see https://github.com/microsoft/monaco-editor/issues/2242.

Maybe there could even be some option to set font size for output text in the yaml, like default: output font size = apparent editor size. But this is just a perhaps-nice-to-have, since you made output font size accessible via css.

Edit: haha, just read that 1 px = 16*rem 😊-- unless set to a different value in the document. This seems to be the case for revealjs slides, where body text has 40px...

coatless commented 8 months ago

@ute Thinking more, we probably want to stay with a flat entry to match with inside cell options.

webr:
   global-cell-options:
       editor-fontsize: '17.5rt'
       editor-word-wrap: 'true'
       editor-theme: 'light'
```{webr-r}
#| editor-fontsize: 17.5


Trying to parse a pre-nested list isn't ideal 😢 

On the name of the yaml key, any preference on:

- `cell-defaults`
- `cell-options`
- `global-cell-options`
- `global-options`
- `global-settings`
- `defaults`
- `config` 

What would be the most expressive yet easily rememberable and not painful to type out? 

---

Regarding the `fontSize` part, ewk! I think that means that `17.5pt` never was being used. C'le vie.
ute commented 8 months ago

@ute Thinking more, we probably want to stay with a flat entry to match with inside cell options.

Trying to parse a pre-nested list isn't ideal 😢

Maybe it is easier with lua. But a less nested list also has its simplicity merits 😊

On the name of the yaml key, any preference on:

  • cell-defaults
  • cell-options
  • global-cell-options
  • global-options
  • global-settings
  • defaults
  • config

What would be the most expressive yet easily rememberable and not painful to type out?

Hmmm - maybe sleep over it. Are there other options than cell options, that one would want to set in the yaml? If you take the same approach and just copy over the user defined options, then I would keep "cell" in the name of the first sub key, if the receiver list is intended to contain everything that one otherwise would feed in as #| comments.

Regarding the fontSize part, ewk! I think that means that 17.5pt never was being used. C'le vie.

In former releases of Monaco, it apparently still was used - dunno. Pyt med det (as Danes would say)

ute commented 8 months ago

I am wondering if it would make sense to declare a parameter that scales editor and output simultaneously in the same way relatively to root font size (with a better name than font-scale) - what do you think? Or separately for editor and output, but also as a scale factor?

```{webr-r}
#| font-scale: 1.5
1 + 1

defining font-scale for monaco seems to work with these additions/changes to `qwebrCreateMonacoEditorInstance`,  but only in html format. In revealjs, root font size is always reported to be 16px, regardless of the the global document font size 😒

``` js
  const rootfontsize = parseInt(window.getComputedStyle(document.body).getPropertyValue('font-size'));
  ...
  fontSize: qwebrOptions['font-scale'] * rootfontsize 

Setting the font size for code output would probably programmatically override user css. Maybe this gives unexpected results for some?

coatless commented 8 months ago

@ute love the font-size trick. With a little bit of googling (I know, old-school), I did come across:

https://github.com/quarto-dev/quarto-cli/discussions/6510#discussioncomment-6725411

Which then lead over to:

https://github.com/gadenbuie/revealjs-text-resizer/blob/main/_extensions/revealjs-text-resizer/revealjs-text-resizer.js

This seems to suggest:

const currentSize = parseFloat(
  getComputedStyle(document.documentElement)
    .getPropertyValue('--r-main-font-size')
);

const newSize = go_up ? currentSize + 2 : currentSize - 2;
document.documentElement.style.setProperty('--r-main-font-size', newSize + 'px');

Maybe this check can be wrapped with:

// Search for reveal class
if (document.body.classList.contains('reveal')) {
    console.log('This page is using reveal.js.');
}

// Search for reveal class by looking for a specific attribute that is unlikely
// to be by chance in the document.
if (document.body.getAttribute('data-reveal-version')) {
    console.log('This page is using reveal.js.');
}

Otherwise, we can default back to using the original scale trick. Thoughts?

coatless commented 8 months ago

Hmmm - maybe sleep over it. Are there other options than cell options, that one would want to set in the yaml? If you take the same approach and just copy over the user-defined options, then I would keep "cell" in the name of the first sub key, if the receiver list is intended to contain everything that one otherwise would feed in as #| comments.

After sleeping on it, only webr cell-level options will likely be specified since document-level options are already being set directly under the webr key, e.g. packages, show-startup-message, ...

So, I think maybe:

For a more knitr-like call, we would want:

https://yihui.org/knitr/options/

ute commented 8 months ago

Yes, this opts-chunk-set is a natural thing to go for 👍. I thought initially and wrongly that it would interfere with the other (quarto) code chunks, but of course it does not, since it is hidden under the webr key.

I got caught by the idea of completely harmonizing font sizes, such that the editor adapts to local size like code output does. It was a kind of p.i.t.a. with reveal.js, until I decided to override the pre font size, which is 0.55em, there.

I'll try another PR. Font size is expressed by scale relative to environment. Yet another thing to sleep over, and might require some kind of "legacy" fixed font size yaml.

ute commented 8 months ago

Wow, you are better at finding the right tricks, I did not see your reply before I engaged in doing the font-size myself. It could have saved me hours of manic tinkering :-)

@ute love the font-size trick. With a little bit of googling (I know, old-school), I did come across:

quarto-dev/quarto-cli#6510 (reply in thread)

Which then lead over to:

https://github.com/gadenbuie/revealjs-text-resizer/blob/main/_extensions/revealjs-text-resizer/revealjs-text-resizer.js

This seems to suggest:

const currentSize = parseFloat(
  getComputedStyle(document.documentElement)
    .getPropertyValue('--r-main-font-size')
);

const newSize = go_up ? currentSize + 2 : currentSize - 2;
document.documentElement.style.setProperty('--r-main-font-size', newSize + 'px');

Maybe this check can be wrapped with:

// Search for reveal class
if (document.body.classList.contains('reveal')) {
    console.log('This page is using reveal.js.');
}

// Search for reveal class by looking for a specific attribute that is unlikely
// to be by chance in the document.
if (document.body.getAttribute('data-reveal-version')) {
    console.log('This page is using reveal.js.');
}

Otherwise, we can default back to using the original scale trick. Thoughts?

After some thinking I would like it best if editor and output have (roughly) the same size by default. Then it corresponds to ordinary code cells. Therefore (currently) I go for changing the size of code cells and output in reveal.js by css, because they seem too small to me also for ordinary code cells.

If I want to make it smaller temporarily, for example I have too much code on a slide and my students have good eyes, I can wrap it in a div that shrinks font size :-). If I want it to be smaller globally, I would override font size in pre -style.

Just realizing that this last paragraph makes a comment obsolete that I made in my PR, about asking quarto to make a similar font-size chunk option 😊

coatless commented 7 months ago

Global options addressed in https://github.com/coatless/quarto-webr/pull/173

First attempt at harmonizing font in: https://github.com/coatless/quarto-webr/pull/175