Skip to content

Conversation

@ryanseddon
Copy link
Contributor

@ryanseddon ryanseddon commented Nov 7, 2018

  • BREAKING CHANGE?

Fixes #147

Description

Previously the dots loader had a velocity prop that didn't really work as expected when changing the value. To align with the spinner loader this PR adds a duration prop which accepts a number in ms, default is 1250ms, to either slow down or speed up the animation.

Detail

This is a breaking change as it removes the velocity prop in favour of the duration prop.

It also heavily refactors the code of the dots loader to follow the same approach used in the spinner loader.

Unfortunately due to our IE11 support we have to do these animations via js which are fairly CPU intensive compared to css animations.

Pre published: https://garden.zendesk.com/react-components/loaders/#!/Dots/3

If anyone has any optimisations I can do I will be forever grateful.

Checklist

  • 👌 design updates are Garden Designer approved (add the
    designer as a reviewer)
  • 💅 view component styling is based on a Garden CSS
    component
  • 🌐 Styleguidist demo is up-to-date (yarn start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • 💂‍♂️ includes new unit and snapshot tests
  • 📒 any new files are included in the packages src/index.js export
  • 📝 tested in Chrome, Firefox, Safari, Edge, and IE11

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 94.92% when pulling 993b0c5 on ryan/dots_loader_duration_prop into b94f5ca on master.

Copy link
Member

@jzempel jzempel left a comment

Choose a reason for hiding this comment

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

dots

hey @ryanseddon not sure if the GIF captures it, but upon closer inspection something about the updated animation feels substantially less smooth than the previous ease-calculated dots.

your branch on left; master branch on right.

@ryanseddon
Copy link
Contributor Author

yeah running a perf trace in chrome it seems the framerate isn't great on this new version, will look into it.

@ryanseddon
Copy link
Contributor Author

Ok I've made the dots loader considerably worse, i'm gonna close this and move along.

@ryanseddon ryanseddon closed this Nov 9, 2018
@ryanseddon ryanseddon deleted the ryan/dots_loader_duration_prop branch November 9, 2018 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants