Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flash of color with css variables theming #267

Closed
TheFedaikin opened this issue Feb 10, 2022 · 4 comments
Closed

Flash of color with css variables theming #267

TheFedaikin opened this issue Feb 10, 2022 · 4 comments

Comments

@TheFedaikin
Copy link

TheFedaikin commented Feb 10, 2022

Good day maintainers of elm-pages!

I was exploring Elm and I am certainly intrigued! I stumbled upon this repo while doing some silly stuff (to get a better grasp of the Elm and its ecosystem), and I think it's a perfect match for what I wanted to try - SSG generator with a lot of batteries for getting the data into my app!

However, SSGs are notoriously known for flash of color and I've been using pattern similar to the one you can see in this blog for ages now. It's very important if you have pretty different theming for accessibility and for people who are sensitive to flash of light. But in elm-pages I couldn't find an easy way of injecting the script before everything - if I do this via load function in index.js it's obviously too slow sometimes, so flash appears to be flaky (it doesn't happen sometimes) but it's still here.

I obviously can fix it by injecting my js with a build script myself: doing a copy of a js file and adding my script tag (or even inlining it) at the top of the html, but I was wondering if there's a better way? I understand the philosophy about Elm and JS interop, but since SSG kind of defies it anyway (you are allowed to manipulate head, instead of just div that you mount your app into) I was wondering if it's possible for the scope of this project.

I am also open to contribute myself, but I am very-very new in Elm, so it'll probably take some time for me.

I have a reproduction for this here - I have dist and dist2 and you can run scripts such as serve and serve:noflash to test the difference.

Thanks in advance, and thank you for this awesome SSG!

Edit: To note in my repro I only use two themes and you can do it with just css, but I usually have 3+ which you can't do with @media queries, because prefers-color-scheme only has light and dark as possible values.

@dillonkearns
Copy link
Owner

Hello @TheFedaikin, thank you for the issue and the background info!

I just added a feature so users can customize the template for their <head> tag in the elm-pages.config.mjs file like this:

https://github.com/dillonkearns/elm-pages-v3-beta/blob/6a067671200f896c2a9b10a31aab307f88943973/examples/docs/elm-pages.config.mjs#L12-L15

This gives the user a lot of control. The API I'm exposing there lets the user define some head tags that even get the full power of being processed by Vite, so you can import TypeScript, SCSS, etc. there, so it's quite powerful and customizable.

My goal is to make it flexible, but also to protect the elm-pages internals to avoid exposing footguns to users that they need to be careful not to misconfigure. So I'm happy with the design in that regard. But I think it does make sense to support adding some head tags before the main compiled Elm file, which the current design doesn't allow for (the head tags from the elm-pages.config.mjs come after the script tag for the compiled Elm code).

Here's the full template:

https://github.com/dillonkearns/elm-pages-v3-beta/blob/6a067671200f896c2a9b10a31aab307f88943973/generator/src/pre-render-html.js#L24-L53

I'm thinking a good way to support that might be add an additional config hook so the user could do something like this:

export default {
  // ...
  headTagsTemplate: (context) => `
<link rel="stylesheet" href="/style.css" />
<meta name="generator" content="elm-pages v${context.cliVersion}" />
`,
  beforeScriptHeadTagsTemplate: (context) => `
<script>console.log('This should run before any other JS');</script>
`,
};

What do you think of that idea?

@dillonkearns
Copy link
Owner

Thinking about this a bit more, since there's a defer attribute in the script tag for the bundled Elm JS script, I'm wondering if it's fine for that script to show up before the others. Deferred scripts will execute in a non-blocking way, whereas an inline <script> tag or a <script> tag without defer on it will execute in a render-blocking way before the deferred scripts. So it seems that the new headTagsTemplate hook I exposed in the v3 beta is sufficient. Here's a helpful reference.

https://addyosmani.com/blog/script-priorities/

@TheFedaikin
Copy link
Author

TheFedaikin commented Nov 5, 2022

Hey @dillonkearns, thanks for a very thorough response!

I like the config idea, that looks exactly what I wanted and a lot more.

About the defer scripts - for my stuff I usually just inline the script for colorSchemes in the head, since it illuminates possibility of flash altogether, so my use-case is satisfied even when I can add it globally to the head. I think this style of headTemplates can simplify naming:

export default {
  // ...
  headTags: {
    after: context => `<meta name="generator" content="elm-pages v${context.cliVersion}" />`,
    before: context => `<script>console.log('This should run before any other JS');</script>`,
  },
}

or even

export default {
  headTags: context => ({
    after: `<meta name="generator" content="elm-pages v${context.cliVersion}" />`,
    before: `<script>console.log('This should run before any other JS');</script>`,
  }),
}

but it's up to you and your preferences, of course!

Should I close this issue now, or when it's getting shipped in official release?

@dillonkearns
Copy link
Owner

Hey @TheFedaikin, see my later comment, I think the before/after script tags aren't needed: #267 (comment).

You can see this if you tweak the elm-pages.config.mjs in the starter repo like this:

  headTagsTemplate(context) {
    return `
<link rel="stylesheet" href="/style.css" />
<meta name="generator" content="elm-pages v${context.cliVersion}" />
<script>
  alert('Hi!');
</script>
`;
  },

The alert will show before page loads (HTML or anything), which means it's run in a blocking way, so you can achieve what you're looking for with the functionality already 👍

I'll close this issue as I think it's now supported.

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

No branches or pull requests

2 participants