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

Corrected spinner animation #4005

Merged
merged 4 commits into from Apr 19, 2019

Conversation

4 participants
@ranbena
Copy link
Contributor

commented Apr 19, 2019

  Demo
Before
After
ezgif com-video-to-gif

The bug

fa-spinner was not meant to spin smoothly but in steps.

And this is important because why? 🙄

I work a lot with Cypress and it's hurting my eyes and crushing my soul! 😱😵😖😭

The fix

Changed to the dedicated 8 step fa-pulse animation.

Tested?

Nope. But it looks riskless 😅

@CLAassistant

This comment has been minimized.

Copy link

commented Apr 19, 2019

CLA assistant check
All committers have signed the CLA.

Show resolved Hide resolved packages/runner/src/header/header.jsx Outdated

ranbena added some commits Apr 19, 2019

@flotwig flotwig merged commit d172e95 into cypress-io:develop Apr 19, 2019

23 checks passed

ci/circleci: Linux lint Your tests passed on CircleCI!
Details
ci/circleci: Mac build Your tests passed on CircleCI!
Details
ci/circleci: Mac lint Your tests passed on CircleCI!
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: desktop-gui-integration-tests-2x Your tests passed on CircleCI!
Details
ci/circleci: driver-integration-tests-3x Your tests passed on CircleCI!
Details
ci/circleci: lint-typescript Your tests passed on CircleCI!
Details
ci/circleci: reporter-integration-tests Your tests passed on CircleCI!
Details
ci/circleci: run-launcher Your tests passed on CircleCI!
Details
ci/circleci: server-e2e-tests-1 Your tests passed on CircleCI!
Details
ci/circleci: server-e2e-tests-2 Your tests passed on CircleCI!
Details
ci/circleci: server-e2e-tests-3 Your tests passed on CircleCI!
Details
ci/circleci: server-e2e-tests-4 Your tests passed on CircleCI!
Details
ci/circleci: server-e2e-tests-5 Your tests passed on CircleCI!
Details
ci/circleci: server-e2e-tests-6 Your tests passed on CircleCI!
Details
ci/circleci: server-e2e-tests-7 Your tests passed on CircleCI!
Details
ci/circleci: server-e2e-tests-8 Your tests passed on CircleCI!
Details
ci/circleci: server-integration-tests Your tests passed on CircleCI!
Details
ci/circleci: server-performance-tests Your tests passed on CircleCI!
Details
ci/circleci: server-unit-tests Your tests passed on CircleCI!
Details
ci/circleci: unit-tests Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla Contributor License Agreement is signed.
Details
@flotwig

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

Thanks for the PR!

@Bkucera

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

@ranbena I see where you're coming from, but if you look at
https://fontawesome.com/v4.7.0/examples/

there are multiple examples of fa-spinner with fa-spin

I have no preference

@Bkucera

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

@ranbena Here's how you can add this today. In support/index.js do:

addGlobalStyle(`
.fa-spinner.fa-spin {
	animation: fa-spin 1s infinite steps(8)
}
`)

function addGlobalStyle(css) {
  var head, style
  head = window.top.document.getElementsByTagName('head')[0]
  if (!head) {
    return
  }
  style = window.top.document.createElement('style')
  style.type = 'text/css'
  style.innerHTML = css
  head.appendChild(style)
}
@ranbena

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2019

@ranbena Here's how you can add this today. In support/index.js do:

addGlobalStyle(`
.fa-spinner.fa-spin {
	animation: fa-spin 1s infinite steps(8)
}
`)

Hehe 10x @Bkucera :)

brian-mann added a commit that referenced this pull request Apr 20, 2019

laurinenas added a commit to laurinenas/cypress that referenced this pull request Apr 28, 2019

Corrected spinner animation (cypress-io#4005)
  | Demo
------------ | -------------
 Before<br /> After | ![ezgif com-video-to-gif](https://user-images.githubusercontent.com/486954/56430388-e538ca00-62ce-11e9-9748-6f0eb249a69d.gif)

### The bug
`fa-spinner` was not meant to spin smoothly but in steps.

### And this is important because why? 🙄
I work a lot with Cypress and it's hurting my eyes and crushing my soul! 😱😵😖😭

### The fix
Changed to the dedicated 8 step `fa-pulse` animation.

### Tested?
Nope. But it looks riskless 😅

laurinenas added a commit to laurinenas/cypress that referenced this pull request Apr 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.