Skip to content

fix: css-only hydration to prevent FOUC (#991)#4856

Merged
jcfranco merged 7 commits intomasterfrom
paulcpederson/991
Jul 9, 2022
Merged

fix: css-only hydration to prevent FOUC (#991)#4856
jcfranco merged 7 commits intomasterfrom
paulcpederson/991

Conversation

@paulcpederson
Copy link
Copy Markdown
Member

Related Issue: #991

(see also #620, #609)

Summary

Because stencil's hydration styles are applied via JavaScript, there is a time before the JavaScript has run where you can see the slotted content flash in before the component is rendered.

A few releases ago stencil allowed you to turn off their hydrated styles entirely via the invisiblePrehydration config flag. This pull request opts out of the injected styles and generates a set of styles for all our components so that they are invisible if they are not hydrated. Because these styles are then in the global css file, they will apply to the page even prior to stencil's loader running at all, ensuring that the FOUC is removed.

There may be a better way to generate this list of components, but currently just reading the components directory and adding a style for each one.

@paulcpederson paulcpederson requested a review from a team as a code owner July 6, 2022 22:24
@github-actions github-actions Bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jul 6, 2022
@driskull
Copy link
Copy Markdown
Member

driskull commented Jul 6, 2022

This is cool. Do we want to try to tackle the components that should be visibility:hidden by default as well? If not, we could do it as a followup. I'm thinking just a JSON file where we can map the components hidden by default.

https://github.com/Esri/calcite-components/search?q=calcite-hydrated-hidden&type=code

@paulcpederson
Copy link
Copy Markdown
Member Author

@driskull yeah, for sure. I didn't totally understand all the calcite hydrated hidden stuff, so I left it as is, but could absolutely just remove the :not([calcite-hydrated]) and get hidden by default for those.

@driskull
Copy link
Copy Markdown
Member

driskull commented Jul 6, 2022

could absolutely just remove the :not([calcite-hydrated]) and get hidden by default for those.

Yep, that would be ideal.

We would just need to maintain a list/map of those components

@@ -0,0 +1,15 @@
const fs = require("fs");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paulcpederson can you use...

import fs from "fs";
import path from "path";

So we get some type safety?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd have to update our package here :. Related to #4700 (comment)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that and got thrown an error about using import outside a module?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooooh, ok that makes sense, I can make that change once that pr goes in.

Comment thread support/hydrationStyles.ts
Comment thread support/hydrationStyles.ts Outdated
(async function () {
const stylePath = path.normalize(`${__dirname}/../src/assets/styles/_hydration.scss`);
const components = await fs.promises.readdir(path.normalize(`${__dirname}/../src/components/`));
const selectors = components.map((component) => `calcite-${component}:not([calcite-hydrated])`);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this would be nice...

const hiddenComponents = new Set<string>(["alert", "input-message", "popover", "tooltip", "tree-item"]);

(async function () {
  const stylePath = path.normalize(`${__dirname}/../src/assets/styles/_hydration.scss`);
  const components = await fs.promises.readdir(path.normalize(`${__dirname}/../src/components/`));
  const selectors = components.map((component) => `calcite-${component}:not([calcite-hydrated])`);
  const selectors = components
    .filter((component) => !hiddenComponents.has(component))
    .map((component) => `calcite-${component}:not([calcite-hydrated])`);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would flash unstyled content prior to javascript running

@paulcpederson
Copy link
Copy Markdown
Member Author

@driskull So after playing with this a bit, even a raw selector like calcite-alert {} will override styles you put in the host stylesheet, so the solutions still end up being pretty gross. The calcite-hydrated-hidden attribute is actually looking a lot cleaner than the alternatives so it may be better to leave it.

Comment thread src/assets/styles/includes.scss Outdated
Copy link
Copy Markdown
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paulcpederson I'm still seeing some modifications on OSX.

diff --git a/src/assets/styles/_hydration.scss b/src/assets/styles/_hydration.scss
index 889c140f0cfc3a198e2c44823b304f523d1939c2..8685c9fe6e945e9b8af25fe029255019cdf1605c 100644
--- a/src/assets/styles/_hydration.scss
+++ b/src/assets/styles/_hydration.scss
@@ -11,6 +11,45 @@ calcite-avatar:not([calcite-hydrated]),
 calcite-block:not([calcite-hydrated]),
 calcite-block-section:not([calcite-hydrated]),
 calcite-button:not([calcite-hydrated]),
+calcite-calcite-accordion:not([calcite-hydrated]),
+calcite-calcite-action-menu:not([calcite-hydrated]),
+calcite-calcite-alert:not([calcite-hydrated]),
+calcite-calcite-block-section:not([calcite-hydrated]),
+calcite-calcite-button:not([calcite-hydrated]),
+calcite-calcite-chip:not([calcite-hydrated]),
+calcite-calcite-color:not([calcite-hydrated]),
+calcite-calcite-color-picker:not([calcite-hydrated]),
+calcite-calcite-combobox:not([calcite-hydrated]),
+calcite-calcite-date-picker:not([calcite-hydrated]),
+calcite-calcite-dropdown:not([calcite-hydrated]),
+calcite-calcite-dropdown-group:not([calcite-hydrated]),
+calcite-calcite-fab:not([calcite-hydrated]),
+calcite-calcite-graph:not([calcite-hydrated]),
+calcite-calcite-icon:not([calcite-hydrated]),
+calcite-calcite-input:not([calcite-hydrated]),
+calcite-calcite-input-date-picker:not([calcite-hydrated]),
+calcite-calcite-input-message:not([calcite-hydrated]),
+calcite-calcite-label:not([calcite-hydrated]),
+calcite-calcite-list:not([calcite-hydrated]),
+calcite-calcite-list-item:not([calcite-hydrated]),
+calcite-calcite-modal:not([calcite-hydrated]),
+calcite-calcite-pagination:not([calcite-hydrated]),
+calcite-calcite-panel:not([calcite-hydrated]),
+calcite-calcite-pick-list:not([calcite-hydrated]),
+calcite-calcite-popover:not([calcite-hydrated]),
+calcite-calcite-popover-manager:not([calcite-hydrated]),
+calcite-calcite-radio-group:not([calcite-hydrated]),
+calcite-calcite-shell:not([calcite-hydrated]),
+calcite-calcite-shell-panel:not([calcite-hydrated]),
+calcite-calcite-stepper:not([calcite-hydrated]),
+calcite-calcite-tab:not([calcite-hydrated]),
+calcite-calcite-tabs:not([calcite-hydrated]),
+calcite-calcite-tile-select:not([calcite-hydrated]),
+calcite-calcite-tile-select-group:not([calcite-hydrated]),
+calcite-calcite-tooltip-manager:not([calcite-hydrated]),
+calcite-calcite-tree:not([calcite-hydrated]),
+calcite-calcite-tree-item:not([calcite-hydrated]),
+calcite-calcite-value-list:not([calcite-hydrated]),
 calcite-card:not([calcite-hydrated]),
 calcite-checkbox:not([calcite-hydrated]),
 calcite-chip:not([calcite-hydrated]),
@@ -27,6 +66,7 @@ calcite-date-picker-month-header:not([calcite-hydrated]),
 calcite-dropdown:not([calcite-hydrated]),
 calcite-dropdown-group:not([calcite-hydrated]),
 calcite-dropdown-item:not([calcite-hydrated]),
+calcite-dropdown-menu:not([calcite-hydrated]),
 calcite-fab:not([calcite-hydrated]),
 calcite-filter:not([calcite-hydrated]),
 calcite-flow:not([calcite-hydrated]),
@@ -88,8 +128,9 @@ calcite-tooltip:not([calcite-hydrated]),
 calcite-tooltip-manager:not([calcite-hydrated]),
 calcite-tree:not([calcite-hydrated]),
 calcite-tree-item:not([calcite-hydrated]),
+calcite-utils:not([calcite-hydrated]),
 calcite-value-list:not([calcite-hydrated]),
 calcite-value-list-item:not([calcite-hydrated]) {
   visibility: hidden;
   pointer-events: none;
-}
+}
\ No newline at end of file

@driskull
Copy link
Copy Markdown
Member

driskull commented Jul 7, 2022

Maybe we could use the docs:json or stats to generate the styles?

https://stenciljs.com/docs/docs-json
https://stenciljs.com/docs/stats

@jcfranco
Copy link
Copy Markdown
Member

jcfranco commented Jul 8, 2022

Maybe we could use the docs:json or stats to generate the styles?

This would be great, but but it would require 2 builds. Let's proceed with the current approach and we can revisit later if we notice any funkiness. 🎶🎸🎶

@paulcpederson
Copy link
Copy Markdown
Member Author

@driskull going to experiment with using a custom target, if that requires two builds I can just add a filter for these old folders

@paulcpederson
Copy link
Copy Markdown
Member Author

Should we gitignore the generated scss file? Seems like the sass files in our project are not distributed as part of a release, so shouldn't be an issue to ignore it as it's generated each build anyways.

@jcfranco
Copy link
Copy Markdown
Member

jcfranco commented Jul 8, 2022

Good question. I think we can ignore it.

Copy link
Copy Markdown
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘
🤘🎸🎸🎸🎸🤘🎸🤘🤘🤘🤘🤘🎸🤘🎸🎸🎸🎸🤘🤘🎸🎸🎸🤘🤘🎸🎸🤘🤘🎸🤘🤘🤘🎸🤘🎸🎸🎸🎸🤘
🤘🎸🤘🤘🎸🤘🎸🤘🤘🤘🤘🤘🎸🤘🎸🤘🤘🤘🤘🎸🤘🤘🤘🤘🎸🤘🤘🎸🤘🎸🎸🤘🎸🎸🤘🎸🤘🤘🤘🤘
🤘🎸🎸🎸🎸🤘🎸🤘🤘🎸🤘🤘🎸🤘🎸🎸🎸🤘🤘🤘🎸🎸🤘🤘🎸🤘🤘🎸🤘🎸🤘🎸🤘🎸🤘🎸🎸🎸🤘🤘
🤘🎸🤘🤘🎸🤘🎸🤘🎸🤘🎸🤘🎸🤘🎸🤘🤘🤘🤘🤘🤘🤘🎸🤘🎸🤘🤘🎸🤘🎸🤘🤘🤘🎸🤘🎸🤘🤘🤘🤘
🤘🎸🤘🤘🎸🤘🤘🎸🤘🤘🤘🎸🤘🤘🎸🎸🎸🎸🤘🎸🎸🎸🤘🤘🤘🎸🎸🤘🤘🎸🤘🤘🤘🎸🤘🎸🎸🎸🎸🤘
🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘

Comment thread support/hydrationStyles.ts Outdated
});
const selectors = components
.filter((entry) => entry.isDirectory())
.filter((entry) => entry.isDirectory() && !entry.name.includes("calcite-"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: I don't think we need this. There shouldn't be any calcite-prefixed folders anymore.

@paulcpederson
Copy link
Copy Markdown
Member Author

paulcpederson commented Jul 8, 2022

@driskull can you try this one more time? I've added a little heuristic to filter out non-component files and folders, so I think this should be pretty accurate now, even for devs with older cached directories

Copy link
Copy Markdown
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug reports for broken functionality. Issues should include a reproduction of the bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants