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

feat: change mui-series pause behavior & add .is-paused for Safari support #97 #108

Merged
merged 4 commits into from May 8, 2018

Conversation

ncoden
Copy link
Contributor

@ncoden ncoden commented May 6, 2018

Bug:

When animation-play-state: paused is set before the animation start, macOS Safari does not start the animation, even after animation-play-state: running is set. If animation-play-state: paused is set later, macOS Safari correctly play and pause the animation.

See #97 (comment) for more informations.

Fix:

The new queue behavior is the following:

  • At first no animation-name is set and the first frame properties are applied. So it looks like the animation is paused at the first frame.
  • Start the animation with .is-animating (set animation-name)
  • Pause the animation with .is-paused (animation-play-state: paused)
  • Restart the animation by removing .is-paused
  • Reset the animation by removing .is-animating

Changes:

  • 13bb2de Change mui-series behavior for better Safari support
    Do not set the animation name until it need to be played. This is the only way to control the animation start on all browsers (including latest Safari versions). See issue for more informations.

  • efd6e38 Add .is-paused mui-series modifier to pause the queue
    As .is-paused is not set when the animation start, macOS Safari handle it correctly and can play and pause the animation.

  • 290855e Add documentation to play, pause and reset mui-series

⚠️ BREAKING CHANGE

  • When .is-animating is removed from .mui-series, the queue is now reset instead of paused. Setting .is-animating back will start the queue from its begining.

    From now: set .is-paused (without removing .is-animating) to pause the queue. Remove both .is-animating and .is-paused to reset the queue.

  • Standard animations names are not supported anymore as mui-series queues names. Make sure you use unique names for your mui-series queues.

    Before

    @include mui-series {
      .shake         { @include mui-queue(3s, 0s, shake); }
      .spin          { @include mui-queue(3s, 0s, spin); }
    }

    After

    @include mui-series {
      .my-serie-shake { @include mui-queue(3s, 0s, shake); }
      .my-serie-spin  { @include mui-queue(3s, 0s, spin); }
    }

ncoden added 3 commits May 6, 2018 22:31


Do not set the animation name until it need to be played. This is the only way to control the animation start on all browsers (including latest Safari versions). See issue for more informations.

**BREAKING CHANGE**
When `.is-animating` is removed from `.mui-series`, the animation is now reset instead of paused. Setting `.is-animating` back will start the animation from its begining.

Closes foundation#97
As `.is-paused` is not set at the animation definition, Safari handle it correctly and can play and pause the animation.

See foundation#97 (comment)
@ncoden
Copy link
Contributor Author

ncoden commented May 6, 2018

Poke @SassNinja @hugogiraudel @DanielRuf for review.
This replaces #107. See #97 (comment).

@SassNinja
Copy link

SassNinja commented May 8, 2018

@ncoden this is awesome! Good job!

However I was facing an issue while testing.
My example code (taken over from docs) didn't work as expected but the shake animation did start immediately before having added is-animating

@include mui-series {
  .shake    { @include mui-queue(3s, 0s, shake); }
  .spin     { @include mui-queue(3s, 0s, spin); }
  .fade-zoom { @include mui-queue(3s, 0s, fade, zoom); }
}

I checked it and have found out there's already a .shake in my motion-ui.css which comes from _classes.scss.
Precisely it's the animations mixin

@mixin motion-ui-animations {
  .shake    { @include mui-animation(shake); }
  .spin-cw  { @include mui-animation(spin); }
  .spin-ccw { @include mui-animation(spin(ccw)); }
  .wiggle   { @include mui-animation(wiggle); }

Since this also adds the animation name to the CSS the animation of course starts immediately. I fixed it by changing the class names in my HTML & Sass

@include mui-series {
  .shakeX ...

Do you think this is something we should care about?
(maybe adjust the docs)

@ncoden
Copy link
Contributor Author

ncoden commented May 8, 2018

animation-play-state: paused cannot be used right away so I don't see any way to support classic animation classes in mui-series. More generally, you mixed classes without knowing it, we cannot expect this to work properly. I can't be upset about my component being invisible after named it .is-hidden 😉.

The documentation example should be modified, but no warning is required. However this modified behavior should be in the migration notes.

Edit: I updated the PR message.

@SassNinja
Copy link

Alright!

Apart from that everything looked good to me 👍

@ncoden
Copy link
Contributor Author

ncoden commented May 8, 2018

@SassNinja Done.

@ncoden ncoden merged commit 0eb9dc7 into foundation:master May 8, 2018
ncoden added a commit to ncoden/motion-ui that referenced this pull request May 21, 2018
# Motion UI v2.0.0

We're happy to release Motion UI 2.0 with a better support of macOS Safari, API improvements, some bug fixes and various maintanance stuff.

## 🚀  Bidirectionals transitions/effects defaults

We changed the default direction of all transition and effect mixins so it depends on the `in` or `out` state. Calling the same mixin with `in` and `out` states now results in the same animation playing forward and backward.

💥 **BREAKING CHANGE**. We changed the effects and transitions API the following way:
* All direction-related options now have opposite values as defauls according to the `in` or `out` state.
* The `slide` effect defaults are now `left` and `right` (according to `$state`) for consistency with the `slide` transition.
* The `zoom` transition and effect defaults are now `0` and `1` (according to `$state`) for consistency with others transitions defaults.

### How to migrate

For the `hinge`, `slide`, `spin`, `zoom` effect functions and `mui-fade`, `mui-hinge`, `mui-slide`, `mui-spin`, `mui-zoom` transition mixins: if `$state: out` is used and no direction parameters are given, manually pass the "forward" parameters to keep them playing forward.

```scss
// Before
@include mui-fade($state: out /*, $from: 0, $to: 1*/);
// After
@include mui-fade($state: out, $from: 0, $to: 1);
```

For the `slide` effect function: if no `$direction` is given, manually pass `$direction: up` to keep the effect sliding to top instead of the new default `left`/`right`.

```scss
// Before
$effect: slide(/* $direction: up */);
// After
$effect: slide($direction: up);
```

For the `mui-zoom` transition mixin: if no `$from` or `$to` are given, manually pass `$from: 1.5` and `$to: 1` to keep the previous behavior.

```scss
// Before
@include mui-zoom(/* $from: 1.5, $to: 1 */);
// After
@include mui-zoom($from: 1.5, $to: 1);
```

## 🚀  New pausing behavior for `mui-queue` for Safari support

With the previous `mui-series` behavior, the serie was paused until the `.is-animating` class was added. Unfortately, the implementation behind this did not work on all macOS Safari versions and was even breaking the whole animation. In order to fully support macOS Safari, we changed the `mui-series` paused
behavior and introduced `.is-paused`.

💥 **BREAKING CHANGE**: When `.is-animating` is removed from `.mui-series`, the queue is now reset instead of paused. Setting `.is-animating` back will start the queue from its begining.

From now you can:
* **Start** the queue by adding `.is-animating`
* **Pause** the queue by adding `.is-paused`
* **Continue** the queue by removing `.is-paused`
* **Reset** the queue by removing `.is-animating`

### How to migrate

If you need an animation to pause midway, add `.is-paused` instead of removing `.is-animating`. For example in jQuery:

```js
// Before
function play($elem) {
  $elem.addClass('is-animating');
}
function pause($elem) {
  $elem.removeClass('is-animating');
}

// After
function play($elem) {
  $elem.addClass('is-animating').removeClass('is-paused');
}
function pause($elem) {
  $elem.addClass('is-paused');
}
function reset($elem) {
  $elem.removeClass('is-animating is-paused');
}
```

As a side-effect of this, standard animations names are not supported anymore as `mui-series` queues names. Make sure you use unique names for your `mui-series` queues.

```scss
// Before
@include mui-series {
  .shake          { @include mui-queue(3s, 0s, shake); }
  .spin           { @include mui-queue(3s, 0s, spin); }
}

// After
@include mui-series {
  .my-serie-shake { @include mui-queue(3s, 0s, shake); }
  .my-serie-spin  { @include mui-queue(3s, 0s, spin); }
}
```

## 🐛  Safer animation keyframes names for CSS

Fixes a bug when using decimals values for the `zoom` effect and transition arguments would generate an invalid CSS Keyframes name and break the animation.

We changed the way we validate arguments and generate keyframe in such a way that they will always have a valid CSS name for all effects, transitions and arguments passed in.

## 📄  All changes

* 💥  foundation#103 - Make transitions/effects directions depending on in/out state (@ncoden, closes foundation#83)
* 💥  foundation#108 - Change mui-series paused behavior for better Safari support (@ncoden, closes foundation#97)
* 🐛  foundation#100 - Fix deprecation warning for Sass 4.0 (@rediris, closes foundation#99)
* 🐛  foundation#102 - Make animation keyframes names safe for CSS (@ncoden, closes foundation#96)
* 🐛  foundation#109 - Ensure `-mui-keyframe-pct` returns a strintg instead of a list (@ncoden, closes foundation#109)
* 💻  foundation#110 - Update development dependencies to latest versions (@ncoden)
* 💻  foundation#112 - Ensure support of iOS Safari 7 and drop support for Android browser 2.3 (@ncoden)
* 💻  foundation#111 - Add Travis to run tests on `node-sass` 3/4/latest (@ncoden)
* 📖  foundation#94  - Fix parameter name for zoom animation (@cmarchena)

## 👩‍💻  Contributors

Thank you to the amazing people who contributed to this release:
* Carlos Marchena (@cmarchena)
* Kevin Ball (@kball)
* Nicolas Coden (@ncoden)
* Roman Edirisinghe (@rediris)
ncoden added a commit to ncoden/motion-ui that referenced this pull request May 21, 2018
We're happy to release Motion UI 2.0 with a better support of macOS Safari, API improvements, some bug fixes and various maintanance stuff.

We changed the default direction of all transition and effect mixins so it depends on the `in` or `out` state. Calling the same mixin with `in` and `out` states now results in the same animation playing forward and backward.

💥 **BREAKING CHANGE**. We changed the effects and transitions API the following way:
* All direction-related options now have opposite values as defauls according to the `in` or `out` state.
* The `slide` effect defaults are now `left` and `right` (according to `$state`) for consistency with the `slide` transition.
* The `zoom` transition and effect defaults are now `0` and `1` (according to `$state`) for consistency with others transitions defaults.

For the `hinge`, `slide`, `spin`, `zoom` effect functions and `mui-fade`, `mui-hinge`, `mui-slide`, `mui-spin`, `mui-zoom` transition mixins: if `$state: out` is used and no direction parameters are given, manually pass the "forward" parameters to keep them playing forward.

```scss
// Before
@include mui-fade($state: out /*, $from: 0, $to: 1*/);
// After
@include mui-fade($state: out, $from: 0, $to: 1);
```

For the `slide` effect function: if no `$direction` is given, manually pass `$direction: up` to keep the effect sliding to top instead of the new default `left`/`right`.

```scss
// Before
$effect: slide(/* $direction: up */);
// After
$effect: slide($direction: up);
```

For the `mui-zoom` transition mixin: if no `$from` or `$to` are given, manually pass `$from: 1.5` and `$to: 1` to keep the previous behavior.

```scss
// Before
@include mui-zoom(/* $from: 1.5, $to: 1 */);
// After
@include mui-zoom($from: 1.5, $to: 1);
```

With the previous `mui-series` behavior, the serie was paused until the `.is-animating` class was added. Unfortately, the implementation behind this did not work on all macOS Safari versions and was even breaking the whole animation. In order to fully support macOS Safari, we changed the `mui-series` paused
behavior and introduced `.is-paused`.

💥 **BREAKING CHANGE**: When `.is-animating` is removed from `.mui-series`, the queue is now reset instead of paused. Setting `.is-animating` back will start the queue from its begining.

From now you can:
* **Start** the queue by adding `.is-animating`
* **Pause** the queue by adding `.is-paused`
* **Continue** the queue by removing `.is-paused`
* **Reset** the queue by removing `.is-animating`

If you need an animation to pause midway, add `.is-paused` instead of removing `.is-animating`. For example in jQuery:

```js
// Before
function play($elem) {
  $elem.addClass('is-animating');
}
function pause($elem) {
  $elem.removeClass('is-animating');
}

// After
function play($elem) {
  $elem.addClass('is-animating').removeClass('is-paused');
}
function pause($elem) {
  $elem.addClass('is-paused');
}
function reset($elem) {
  $elem.removeClass('is-animating is-paused');
}
```

As a side-effect of this, standard animations names are not supported anymore as `mui-series` queues names. Make sure you use unique names for your `mui-series` queues.

```scss
// Before
@include mui-series {
  .shake          { @include mui-queue(3s, 0s, shake); }
  .spin           { @include mui-queue(3s, 0s, spin); }
}

// After
@include mui-series {
  .my-serie-shake { @include mui-queue(3s, 0s, shake); }
  .my-serie-spin  { @include mui-queue(3s, 0s, spin); }
}
```

Fixes a bug when using decimals values for the `zoom` effect and transition arguments would generate an invalid CSS Keyframes name and break the animation.

We changed the way we validate arguments and generate keyframe in such a way that they will always have a valid CSS name for all effects, transitions and arguments passed in.

* 💥  foundation#103 - Make transitions/effects directions depending on in/out state (@ncoden, closes foundation#83)
* 💥  foundation#108 - Change mui-series paused behavior for better Safari support (@ncoden, closes foundation#97)
* 🐛  foundation#100 - Fix deprecation warning for Sass 4.0 (@rediris, closes foundation#99)
* 🐛  foundation#102 - Make animation keyframes names safe for CSS (@ncoden, closes foundation#96)
* 🐛  foundation#109 - Ensure `-mui-keyframe-pct` returns a strintg instead of a list (@ncoden, closes foundation#109)
* 💻  foundation#110 - Update development dependencies to latest versions (@ncoden)
* 💻  foundation#112 - Ensure support of iOS Safari 7 and drop support for Android browser 2.3 (@ncoden)
* 💻  foundation#111 - Add Travis to run tests on `node-sass` 3/4/latest (@ncoden)
* 📖  foundation#94  - Fix parameter name for zoom animation (@cmarchena)

Thank you to the amazing people who contributed to this release:
* Carlos Marchena (@cmarchena)
* Kevin Ball (@kball)
* Nicolas Coden (@ncoden)
* Roman Edirisinghe (@rediris)
@ncoden ncoden mentioned this pull request May 21, 2018
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants