asciidoctor-vscode icon indicating copy to clipboard operation
asciidoctor-vscode copied to clipboard

Custom stylesheet defined inline not working in 3.0.0-prerelease

Open danyill opened this issue 2 years ago • 12 comments

This is an example issue. Use just the headings and fill out all necessary information. Screenshots & Files and Additional Context are optional.

Description

My custom stylesheet does not work in 3.0.0-prerelease:

image

When I uninstall the prerelease and switch back to the release version 2.9.8 it works:

image

In each case I have the following settings:

image

System Information

Version: 1.68.1 (user setup)
Commit: 30d9c6cd9483b2cc586687151bcbcd635f373630
Date: 2022-06-14T12:48:58.283Z
Electron: 17.4.7
Chromium: 98.0.4758.141
Node.js: 16.13.0
V8: 9.8.177.13-electron.0
OS: Windows_NT x64 10.0.19044

To Reproduce

Steps to reproduce the issue:

  1. Open a folder.

  2. Create a document, something like:

    = Test
    :stylesheet: styles.css
    
    Hello world
    
    `hey`
    
    What is happening?
    
    == Something is not right
    
    This is not green.
    
  3. With a css file in the same folder, styles.css

    body {
        font-family: "Times New Roman";
        color: green !important;
    }
    
    @media (prefers-color-scheme: dark) {
    body { background:  #333; color: white; }
    }
    
    @media (prefers-color-scheme: light) {
        body { background:  #ddd; color: black; }
    }
    
    p code {
    color: darkblue;
    }
    

Screenshots & Files

See above.

Additional Context

I think that this attribute is being overridden and that our selection logic for a custom stylesheet is incorrect.

danyill avatar Jul 06 '22 20:07 danyill

This logic was removed entirely, we are now using the same strategy/code as the Markdown extension (which I believe is more sound and should work in VS Code Web as well).

I left a question/comment in the code but I'm not convinced that we should support styledir / stylesheet in the preview: https://github.com/asciidoctor/asciidoctor-vscode/blob/master/src/asciidoctorWebViewConverter.ts#L383

ggrossetie avatar Jul 06 '22 20:07 ggrossetie

This logic was removed entirely, we are now using the same strategy/code as the Markdown extension (which I believe is more sound and should work in VS Code Web as well).

I left a question/comment in the code but I'm not convinced that we should support styledir / stylesheet in the preview: https://github.com/asciidoctor/asciidoctor-vscode/blob/master/src/asciidoctorWebViewConverter.ts#L383

I may have missed that...

I'd like to support it just because it allows me to specify a stylesheet within a workspace without changing the settings or forcing a consistency across workspaces.

danyill avatar Jul 06 '22 21:07 danyill

You can configure the asciidoc.styles setting per workspace if you want.

ggrossetie avatar Jul 06 '22 21:07 ggrossetie

Sure, there is a solution which is good.

I'd like to think that a vanilla set of Asciidoc options would just work and the document writer wouldn't need to accommodate the needs of the editor insofar as possible if they were using standard syntax. Otherwise we need a bit of a disclaimer somewhere in the README.

What is your concern? Is it making it work with the webview or complexity to support multiple options?

danyill avatar Jul 07 '22 03:07 danyill

I'd like to think that a vanilla set of Asciidoc options would just work and the document writer wouldn't need to accommodate the needs of the editor insofar as possible if they were using standard syntax. Otherwise we need a bit of a disclaimer somewhere in the README. What is your concern? Is it making it work with the webview or complexity to support multiple options?

To be honest, both. What would be the precedence/priority order if multiple options are defined?

In the future, we might want to support Antora UI bundles/styles and it will get even more complicated.

In my mind, stylesheet and stylesdir should be used when you publish/export your document to HTML, not when you are previewing it. Arguably, some users might want to use VS Code dark mode or a custom theme and use another theme or the built-in Asciidoctor style when publishing to HTML.

The stylesdir could also reference the path where you will publish your HTML files whereas the preview will resolve path relative to the AsciiDoc file. So making a clear distinction avoid confusion.

I'm still open to support the stylesheet attribute but ignore the stylesdir but as mentioned above it might create even more confusion...

ggrossetie avatar Jul 07 '22 16:07 ggrossetie

Thanks for explaining. I am not trying to have a debate for its own sake...

I feel strongly about this only because I think standard language features shouldn't be discarded in favour of editor configuration. It feels like then we are creating (even if it's only a preview) a mini-language-variant and forcing it upon the user because they're using "our" editor.

That's a bit of a caricature but perhaps there is a little truth in it (I have not looked at what IntelliJ and Atom do).

(If that was true) I would say we would prefer to drop stylesheet configuration via settings and rely on the language builtins. The AsciiDoc language should be simple enough that we don't need to wrap configuration options around it to handle builtins (that defeats the purpose and this is a point often made that wrapping too much GUI around the plain text is counter to the objective of having users learn the language).

But I do like the idea of being able to configure an editor theme for the html (to make it more compact or better suited to a side view).

For myself using the configuration (preview.style) is fine and is a suitable workaround.

I guess I'm saying it seems like all these options are there for good reasons.

So I'm happy for you to decide and I don't think it's urgent (but perhaps dropping support for this should be included in the 3.0.0 release notes even if we add it back in later).

What would be the precedence/priority order if multiple options are defined?

Does the following order/precedence make sense?

:stylesheet: / :stylesdir: preview.style preview.useEditorStyle outcome comment
0 0 0 default Asciidoctor stylesheet  
0 0 1 VS Code stylesheet  
X 1 X specified stylesheet in config Editor overrides document ("user requires particular outcome specified by configuration for editing environment")
1 0 X inline specified stylesheet Support in-built syntax unless editor requirements provided
  • X = don't care
  • 1 = set
  • 0 = unset

In the future, we might want to support Antora UI bundles/styles and it will get even more complicated.

If a playbook or UI is specified that overrides all things I'd say by default (similar to a configuration defined stylesheet) by default but with a configuration toggle option to treat it just like "normal Asciidoc file" (and maybe a popup, "Did you want to view this using the Antora theme or using the editor configuration?"

danyill avatar Jul 07 '22 21:07 danyill

Does the following order/precedence make sense?

I think that definitely makes sense. I would definitely prefer the AsciiDoc extension to preserve my stylesheet by default and only apply its own if I explicitly ask it to. After all, the extension's preview allows one to not just get instant feedback on the content of the adoc file, but on all the other aspects as well: document attributes, docinfo, source highlighting theme, etc. It seems weird to exclude stylesheets from this feedback — in theory you use the AsciiDoc extension as an AsciiDoc-specific version of Live Server (one that doesn't require you to :linkcss: to changes to your stylesheet).

rben01 avatar Jul 08 '22 16:07 rben01

The main issue is that we are always linking to the stylesheet, to comply with VS Code security model and compatibility with VS Code for the Web. And, we are also linking from the AsciiDoc file and not from the published location which may or may not be the same.

In other words, the linkcss attribute has no effect and using stylesdir and stylesheet attributes to link to a stylesheet might not produce the expected result.

I also believe that style options should not be defined in AsciiDoc files. As mentioned above, you might want to use a different stylesheet depending on the context.

I feel strongly about this only because I think standard language features shouldn't be discarded in favour of editor configuration. It feels like then we are creating (even if it's only a preview) a mini-language-variant and forcing it upon the user because they're using "our" editor.

I don't necessarily agree. Presentation-related options (such as iconfont-remote, webfonts, stylesheet, stylesdir, highlightjsdir...) are tightly tied to the environment/converter. For instance, it might not be possible to read from the file system or from the network and some options might not be applicable.

The preview is not an accurate reflection of the published result.

ggrossetie avatar Jul 17 '22 12:07 ggrossetie

Otherwise we need a bit of a disclaimer somewhere in the README

Indeed! I also think we should state key differences and limitations with the built-in HTML 5 converter. We are are using Highlight.js 11.x (and not Highlight.js 9.x), only Highlight.js is supported as a syntax highlighter, and so on...

ggrossetie avatar Jul 17 '22 12:07 ggrossetie

If I might add my 2 cents. Supporting stylesheet and stylesdir in the preview is really helpful in assuring unified experience between all the team members that modify the same document (especially because some of them us VSCode and some IntelliJ IDEA, where the preview works perfectly fine). If I decide to make the color of the headers to be green instead of blue, I'd like all the people that work with the document to see that the headers are green, without having to distribute the new stylesheet to all of them (or to the new team members that just start working with the file).

The preview is not an accurate reflection of the published result.

To be honest, from my experience, preview is the exact representation of of the published result. Even if I end up delivering PDF instead of HTML, my PDF styles are constructed specifically to mirror the HTML and by extension - the preview. One can ask - if preview is not an accurate reflection of the published result then what is? :) I need to somehow know how my document is gonna look like once I release it.

Please, reconsider restoring the support for in-document stylesheet configuration. Maybe we could have the flag similar to Asciidoc > Preview: Use Editor Style but instead it would be Asciidoc > Preview: Prioritize In-Document Style

GauntletPL avatar May 08 '24 08:05 GauntletPL