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

criticalStyles HOC #228

Closed
wants to merge 1 commit into from
Closed

criticalStyles HOC #228

wants to merge 1 commit into from

Conversation

George-Payne
Copy link

@George-Payne George-Payne commented Mar 18, 2017

Hello,

I was having an issue with #147, especially for show / hide and animations, so I wrote this workaround (fix?).
This probably isnt a contender for being merged, as it is specifically a workaround for react, and requires it as a dependancy. But I thought it might be useful for someone, or maybe for an "react-aphrodite" repo of some kind.

criticalStyles higher order component.

Can be imported via:

import { criticalStyles } from 'aphrodite';
// or 
import { criticalStyles } from 'aphrodite/no-important';

and allows you to provide any number of critical styles to be added to the stylesheet before render.

Usage

Can be used to create critical styles either before render, or on require.

Single style:

// styles passed raw
criticalStyles(styles.needed_style)(ComponentName);

This results in the equivelent of css(styles.needed_style)

Multiple styles:

// styles passed raw
criticalStyles(styles.needed_style, styles.more_styles)(ComponentName);

This results in the equivelent of css(styles.needed_style) css(styles.more_styles) and is intended for multiple classes within the same component.

Multiple styles in one class:

// styles passed in an array
criticalStyles(
     [styles.base_style, styles.extension_styles],
)(ComponentName);

// (obviously you can also do this multiple times)
criticalStyles(
     [styles.base_style, styles.extension_styles],
     [styles.base_also, styles.extension_also]
)(ComponentName);

This results in the equivelent of css(styles.base_style, styles.extension_styles).

Styles dependant on props.

// styles returned from a function 
criticalStyles(
     props => styles[props.myStyle],
)(ComponentName);

// Once again you can also use an array to join styles, and do this multiple times
criticalStyles(
     props => styles[props.myStyle],
     props => ([ styles.base_style, styles[props.myStyle] ])
)(ComponentName);

All previous ways add the style on require, however, as props arent known on require, this will delay rendering of the component until the styles are added to the sheet.

Mixed

// you can use all or any of the above at the same time
criticalStyles(
     [styles.base_style, styles.extension_styles],
     props => ([ styles.base_style, styles[props.myStyle] ]),
     styles.needed_style,
)(ComponentName);

Some examples:

(All screen captures have been paused on modification of stylesheet, as framerate was too low to show flash of unstyled content)

Rendering a previously unseen component:

Before:

export default IconLink;

IconLink before

The component enters unstyled.

After:

export default criticalStyles(styles.link, styles.text_input)(IconLink);

IconLink after

The component enters with the minimum amount of styling to be viable already in the stylesheet, as specified.

Transitions.

Before:

export default Bubble;

Bubble before

The component enters unstyled, and is styled too late to transition to its final state. It exits unstyled, once again too late to transition. Subsiquent transitions work as expected.

After:

export default criticalStyles(
  props => ([styles.base, styles[props.location], styles.exited]),
  props => ([styles.base, styles[props.location], styles.entering]),
  props => ([styles.base, styles[props.location], styles.entered]),
  props => ([styles.base, styles[props.location], styles.exiting]),
)(Bubble);

Bubble after

All animation styles are applied before the component is rendered, allowing it to transition between each.

- George

@khanbot
Copy link

khanbot commented Mar 18, 2017

Hey @George-Payne,

Thanks for the PR! Mind signing our Contributor License Agreement?

When you've done so, go ahead and comment [clabot:check] and I'll check again.

Yours truly,
khanbot

@George-Payne George-Payne changed the title criticalStyles HOC and export criticalStyles HOC Mar 18, 2017
@George-Payne
Copy link
Author

[clabot:check]

@khanbot
Copy link

khanbot commented Mar 18, 2017

CLA signature looks good 👍

@xymostech
Copy link
Contributor

Hi @George-Payne. Thanks for submitting your pull request! Unfortunately, I don't think that this is the solution that we want to use to solve this problem. Aphrodite tries pretty hard to not depend on React, so I don't think that we'd want to add that hard dependency that you noted for this issue that few people are running into.

I think there have been a couple of solutions to this problem stewing, notably exposing the flushToStyleTag() function and switching to using insertRule to make styles be inserted faster. If you'd like to contribute, maybe try your hand at implementing one of those fixes? (I have some code sitting around to do add insertRule support but haven't gotten around to getting it out.)

This code might be the solution someone is looking for, though! Even if it's not in core Aphrodite, putting it up somewhere could be useful for others. Thanks for contributing back.

@xymostech xymostech closed this Mar 20, 2017
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

Successfully merging this pull request may close these issues.

3 participants