feat(runtime-dom): adjust transition class process by luwuer · Pull Request #2597 · vuejs/core

luwuer · 2020-11-12T11:53:12Z

Make sure that *-enter/leave-from is the beginning of transition.

luwuer · 2020-11-13T01:49:24Z

May i ask is there any information of reviewing time? @LinusBorg @posva

I need you help to verify if I've handled both issues correctly.

posva · 2020-11-30T08:04:47Z

These existing test cases should not change. That behavior is correct

I've changed some of transform logic at Transition.ts , so that the test cases should follow the changes. I'm going to analyze it a little bit that why in this way.

What I mean is that the fix should not require to change those tests

I'll explain why I'm doing this and please wait a minute.

luwuer

before modification

The transition class test-enter-active and test-enter-from are added to el in onBeforeEnter, and then the class test-enter-from is removed in next tick(animation frame).

For example, Reproduction link of issue #2531.

The el has a default style opacity: 1.0 mdn
We set opacity: 0 and transition to the el when beforeEnter, so the el will start transition from opacity 1.0 to opacity 0.
But we remove opacity: 0; from el by remove class .tooltip-enter-from in the next frame, so transition stop.

So that the main problem in these cases is that .tooltip-enter-from is not the initial style.

.tooltip-enter-from,
.tooltip-leave-to {
  transform: translateY(-30px) scale(0.96);
  opacity: 0;
}

after modification

The transition class test-enter-active is added to el in 'onEnter' which is triggered next tick.

Also Reproduction link of issue #2531.

The el has a default style opacity: 1.0
We set opacity: 0 to el when beforeEnter, but the el will not start transition because of no css transition property.
Then we set transition and opacity: 0 to the el in the next frame, the el start transition from opacity 0 to opacity 1.0.

The same as leave.

summary

I changed tests because of i think the .tooltip-enter/leave-active should be set in the next frame.

luwuer · 2020-12-01T09:59:52Z

I'll explain why I'm doing this and please wait a minute.

posva · 2020-12-01T11:06:32Z

What I meant is that -active should be set on the first frame, as it is right now. The change you want to do is a breaking change and won't happen

luwuer · 2020-12-01T12:08:50Z

What I meant is that -active should be set on the first frame, as it is right now. The change you want to do is a breaking change and won't happen

Well, i am not understand why its breaking change, there has no change about external APIs.

I think about it another way just now:

Add a workaround class at the same time when add -active on the first frame.
Remove the workaround class on the beginning of the next frame.

Is that what we need ?

posva · 2020-12-01T12:41:41Z

Maybe this is clearer: https://v3.vuejs.org/guide/transitions-enterleave.html#transition-classes
Changing how the classes are added is a breaking change

luwuer · 2020-12-01T13:05:32Z

Thanks for your replying. According to the document, its really a breaking change.

I have aslo a question: If i use a workround class to fix it and its also have to change tests(because a new class has been added on the first frame), is that a breaking change ?

Just like below:

    onBeforeEnter(el) {
      onBeforeEnter && onBeforeEnter(el)
      addTransitionClass(el, enterActiveClass)
      addTransitionClass(el, enterFromClass)
      // add
      addTransitionClass(el, 'transition-workaround')
      nextFrame(() => {
        removeTransitionClass(el, 'transition-workaround')
      })
    },

Or change inline style.

luwuer · 2020-12-02T09:48:11Z

How about this way @posva

Disable transition on the first frame and enable it on the next frame.

luwuer

This is the reason of why changed seemingly unrelated tests.

luwuer · 2020-12-02T10:01:32Z

Why add await nextFrame() ?

const buffer = 5 const transitionFinish = (time = duration) => timeout(time + buffer)

Because the 5 milliseconds can not ensure the next frame refresh.

Why there is no problem before ?
Because the leave transition will be broken when effect on Suspense component. Tell me if need a reproduction link.

yyx990803 · 2020-12-02T18:04:04Z

This (and disableTransition calls in beforeEnter and beforeAppear) seems unnecessary. When elements get a class in a none-rendered state (display: none or off-DOM) they do not trigger transitions anyway.

Oh ok I see you are doing this so it can support visibility toggles.

Testing this locally, I find that this actually alters the behavior of v-show transitions when you hit toggle really fast.

The current behavior smoothly transitions half-way into the opposite direction, but with this change it hard resets to the enter from state.

)

…om" (vuejs#2593)

…n the first frame (vuejs#2531, vuejs#2586)

yyx990803 · 2020-12-02T19:42:24Z

Note: I removed the transition disabling for enter/appear transitions because it breaks v-show's current behavior. Whether we should or how to support visibility: hidden in transitions should be a separate discussion/PR.

luwuer changed the title ~~feat(runtime-dom): transition component effect on visibility (#2531), transition component does not work with "*-leave-from" (#2593)~~ feat(runtime-dom): adjust transition class process Nov 12, 2020

luwuer force-pushed the adjust-transition-process branch from cb3926d to d0dc826 Compare November 12, 2020 12:05

posva reviewed Nov 30, 2020

View reviewed changes

luwuer commented Dec 1, 2020

View reviewed changes

luwuer force-pushed the adjust-transition-process branch from 786ae54 to 63da830 Compare December 2, 2020 09:43

luwuer commented Dec 2, 2020

View reviewed changes

yyx990803 requested changes Dec 2, 2020

View reviewed changes

luwuer and others added 4 commits December 2, 2020 13:49

feat(runtime-dom): transition component effect on visibility (vuejs#2531

7762bee

)

fix(runtime-dom): transition component does not work with "*-leave-fr…

0987bb5

…om" (vuejs#2593)

feat(runtime-dom): transition component disable transition property o…

3b4198e

…n the first frame (vuejs#2531, vuejs#2586)

refactor: do not disable transition in before enter

c44baae

yyx990803 force-pushed the adjust-transition-process branch from 63da830 to c44baae Compare December 2, 2020 19:34

yyx990803 merged commit e2618a6 into vuejs:master Dec 2, 2020

edison1105 mentioned this pull request Dec 3, 2020

FLIP Animation not applied when first item spliced #2712

Closed

ozguruysal mentioned this pull request Dec 3, 2020

Transitions doesn't work with leave hooks #2720

Closed

Uh oh!

Uh oh!

Conversation

luwuer commented Nov 12, 2020 • edited by posva Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luwuer commented Nov 13, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luwuer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

before modification

after modification

summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

posva commented Dec 1, 2020

Uh oh!

luwuer commented Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

posva commented Dec 1, 2020

Uh oh!

luwuer commented Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luwuer commented Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luwuer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yyx990803 commented Dec 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

luwuer commented Nov 12, 2020 •

edited by posva

Loading

luwuer left a comment •

edited

Loading

luwuer commented Dec 1, 2020 •

edited

Loading

luwuer commented Dec 1, 2020 •

edited

Loading

luwuer commented Dec 2, 2020 •

edited

Loading