Skip to content

Timeout and color config, fix dimensions, pull in loaders.css, SSR, currentColor, spinnerName#58

Merged
KyleAMathews merged 7 commits into
KyleAMathews:masterfrom
merrywhether:pr
May 22, 2017
Merged

Timeout and color config, fix dimensions, pull in loaders.css, SSR, currentColor, spinnerName#58
KyleAMathews merged 7 commits into
KyleAMathews:masterfrom
merrywhether:pr

Conversation

@merrywhether
Copy link
Copy Markdown
Collaborator

@merrywhether merrywhether commented May 17, 2017

This PR addresses a few issues:

  • Add Timeout As Prop #33 : Since the fade in is set as a CSS animation, I couldn't do full configurability, but added 2 smaller intervals of fadeIn timing (half second and quarter second). This includes transitioning away from the noFadeIn prop with a deprecation warning.
  • Coloring the spinners #37 : This allows colorizing the spinners via props and inline styling for easier use. There were 2 spinners that weren't compatible with this approach because they relied on before/after pseudo-elements, and there are appropriate warnings for people who attempt to do that.
  • Dimensions off by default? #48 : A few of the spinners had misconfigured dimensions on the containing div, resulting in difficulty centering them via flexbox, etc. Their dimensions have been adjusted accordingly.
  • loaders.css #14 : Pulls in loaders.css for more spinners, with efforts to minimize package size growth. I omitted spinners from that repo that were very similar to pre-existing "native" spinners.
  • requiring in node raises error #20 / Invalid or unexpected token @-webkit-keyframes sk-fade-in #51 : Made style imports dynamic based on environment variable, allowing server side rendering without lots of hacks. This is similar to react's dynamic behavior based on process.env.PRODUCTION and is a specific enough variable that it shouldn't collide.

Additionally, updated tests, README, and demo site for all new features, and made changes to the test file to appease eslint (change to .jsx, convert tabs to spaces, etc) as well as organizational improvements (describe blocks to group related tests).

All changes were tested inside of a working app for functionality and proper centering alignment.

Comment thread css/wandering-cubes.css
75% { -webkit-transform: translateX(0px) translateY(22px) rotate(-270deg) scale(0.5) }
25% { -webkit-transform: translateX(42px) rotate(-90deg) scale(0.5) }
50% { -webkit-transform: translateX(42px) translateY(42px) rotate(-180deg) }
75% { -webkit-transform: translateX(0px) translateY(42px) rotate(-270deg) scale(0.5) }
Copy link
Copy Markdown
Collaborator Author

@merrywhether merrywhether May 17, 2017

Choose a reason for hiding this comment

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

Not sure why these weren't on the same 42px scale as the non-vendor-prefixed keyframes, so I adjusted them. If there was a reason, I'm happy to revert.

Comment thread css/three-bounce.css
@@ -1,3 +1,7 @@
.sk-three-bounce {
height: 18px;
}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

user-agent default styles added padding to the bottom of this div that affected proper vertical alignment.

@merrywhether merrywhether force-pushed the pr branch 2 times, most recently from 3ea413d to 75f5640 Compare May 17, 2017 05:56
@merrywhether
Copy link
Copy Markdown
Collaborator Author

merrywhether commented May 17, 2017

Second commit refactors everything to be classed-divs with anonymous children divs so that code can be DRYed, relying heavily on the suggestion in #27. This should make on-boarding future loader repos easier. Also makes it so that all spinners accept colorization. Additionally, fixed a bug in the CSS for the folding-cube.

Also does a similar thing for the demos page, using iteration to render all spinners (with colored versions too).

Comment thread css/loaders-css.css
@@ -0,0 +1,52 @@
.ball-triangle-path > div,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Minor overrides to loaders.css styles to allow coloring under new scheme.

@merrywhether
Copy link
Copy Markdown
Collaborator Author

merrywhether commented May 17, 2017

This also addresses #53 indirectly.

@merrywhether
Copy link
Copy Markdown
Collaborator Author

merrywhether commented May 17, 2017

Since this represents a lot of changes, pulled in #19 as well, and maybe this could result in a 3.0 release.

@merrywhether merrywhether changed the title Timeout and color config, fix dimensions, pull in loaders.css, SSR Timeout and color config, fix dimensions, pull in loaders.css, SSR, currentColor, spinnerName May 17, 2017
@KyleAMathews
Copy link
Copy Markdown
Owner

This is awesome! Thanks for giving things a nice cleanup / upgrade! Adding colors support + loaders.css is 💯

Merging and releasing a 3.0.

The next frontier I see for this module is adding tree shaking support. That or perhaps switch to a lerna monorepo approach where there's a package per spinner to keep the size of the package to a minimum.

@KyleAMathews KyleAMathews merged commit 16d7a7d into KyleAMathews:master May 22, 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.

2 participants