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

Unstyled components flashing in Safari #226

Closed
humphreybc opened this issue Jan 16, 2018 · 7 comments
Closed

Unstyled components flashing in Safari #226

humphreybc opened this issue Jan 16, 2018 · 7 comments
Labels

Comments

@humphreybc
Copy link

Description of problem

When using Safari, unstyled components will flash for a split second before becoming styled. This applies to the whole page on initial load as well as components that are rendered when they're opened (like dropdowns and modal dialogs).

Versions

  • OS X 10.13.2
  • React 16.2.0
  • Safari 11.0.2
  • Typestyle 1.6.0

Video

unstyled

App index

import { App } from "dovetail/app";
import * as React from "react";
import * as ReactDOM from "react-dom";
import { forceRenderStyles } from "typestyle";

const container = document.getElementById("react-app");
ReactDOM.render(<App />, container);

forceRenderStyles();
@basarat
Copy link
Contributor

basarat commented Jan 22, 2018

I can see you are calling forceRenderStyles. That is exactly for preventing this unsightly flash : https://typestyle.github.io/#/core/tip-forcerenderstyles-

Suspect there are calls to typestyle.style that aren't a part of the initial render and dependent on some UI state transition / backend response / react lifecycle or something else that is time sensitive. Try moving those styles into namespaces instead of depending on React lifecycle so they are resolved before the dom gets rendered : https://typestyle.github.io/#/core/tip-code-organization

A small reproducible example would be needed if issue persists 🌹

@bradleyayers
Copy link
Collaborator

What do you think about using a microtask scheduler instead of RAF for style()?

@basarat
Copy link
Contributor

basarat commented Jan 22, 2018

What do you think about using a microtask scheduler instead of RAF for style()

It turns in a time war. Who wins will still be determined by how other libraries / user code etc behaves.

@bradleyayers
Copy link
Collaborator

It turns in a time war. Who wins will still be determined by how other libraries / user code etc behaves.

My understanding is that microtasks block the current paint (and as such are guaranteed to execute before the paint). This would guarantee that the styles are added to the DOM and incorporated into the CSSOM before the paint occurs, which would guarantee the styles introduced by a style() call would be present in the next paint.

Your point about other libraries or user code is interesting, but after mulling over it for a while I can't think of how they could interfere with this, as they can't unschedule microtasks.

In my mind the trade-offs to choose between are:

  1. incur worse performance (i.e. potentially < 60fps) by forcing a DOM update before the paint, or
  2. have improved performance, but with potentially broken behaviour

For me option 1 seems a little better than 2. It still gives the user the opportunity to schedule via RAF if they wish (by calling style() in a RAF callback).

@basarat
Copy link
Contributor

basarat commented Jan 23, 2018

Probably an easy fix in user code as import('./someLazyLoadedAppSection').then(forceRenderStyles). I just don't want to do anything to fix a problem I haven't experienced ¯\_(ツ)_/¯ 🌹

Quick notes:

@bradleyayers
Copy link
Collaborator

Fair enough, thanks for the quick reply. For now user code sounds like the way to go. If other people start hitting this problem (unlikely at this point) we can revisit baking something into the library.

Regarding forceRenderStyles, does it only render styles that haven't been applied yet?

@basarat
Copy link
Contributor

basarat commented Jan 23, 2018

, does it only render styles that haven't been applied yet?

No. Just writes the styles to the tag without any smarts :

target.textContent = this.getStyles();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants