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

fix(vmenu): sets bottom style property when using top prop #14239

Open
wants to merge 3 commits into
base: v2-stable
Choose a base branch
from

Conversation

Kevin-H2R
Copy link
Contributor

@Kevin-H2R Kevin-H2R commented Oct 6, 2021

fix #8454
Disclaimer: I noticed this issue was closed only after working on it. Still made a PR as this is my first contribution ever and I guess it does not cost too much. I'd understand if this PR has to be refused.

Description

When double clicking quickly the activator of a v-menu with scale transition and top prop, it reopened lower than it should. The reason of this behavior is that the closing animation of the "first" v-menu is not finished when calculating the top style property of the "second" one.

To fix that, when using top prop, I set the bottom style property instead of the top one, using the activator top position which makes it consistent regardless of the animation. I added a promise on transitionend event, and only updates the dimensions once it resolves.

fixes #8454

Motivation and Context

The change itself helps fixing an UI behavior (#8454)

How Has This Been Tested?

visually

Markup:

// Paste your FULL Playground.vue here
<template>
  <v-container>
    <v-layout wrap justify-space-around style="margin-top: 300px">
      <v-menu
        transition="scale-transition"
        origin="bottom left"
        top fixed
        nudge-top="30px" nudge-right="32px"
      >
        <template v-slot:activator="{ on }">
          <v-btn
            color="primary"
            dark
            v-on="on"
          >
            Scale Transition
          </v-btn>
        </template>

        <v-list>
          <v-list-item
            v-for="(item, i) in items"
            :key="i"
          >
            <v-list-item-title>{{ item.title }}</v-list-item-title>
          </v-list-item>
        </v-list>
      </v-menu>


    </v-layout>
  </v-container>
</template>

<script>
  export default {
    data: () => ({
    items: [
      { title: 'Click Me' },
      { title: 'Click Me' },
      { title: 'Click Me' },
      { title: 'Click Me 2' },
    ],
    }),
  }
</script>

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any features but makes things better)

Checklist:

  • The PR title is no longer than 64 characters.
  • The PR is submitted to the correct branch (master for bug fixes and documentation updates, dev for new features and backwards compatible changes and next for non-backwards compatible changes).
  • My code follows the code style of this project.
  • I've added relevant changes to the documentation (applies to new features and breaking changes in core library)

@Tofandel
Copy link
Contributor

This is just a workaround of one particular case and not an actual fix

To fix this the transition needs to be stopped before recalculating the position

@Kevin-H2R
Copy link
Contributor Author

Updated my fix to follow @Tofandel comment.

@Tofandel
Copy link
Contributor

Tofandel commented Oct 14, 2021

Looks much better but It doesn't solve the issue for me, I think you need to track the transition start and end independently and wait for the end before starting the transitition

Maybe with a counter, everytime transition starts increase and when it ends decrease, watch for the counter reaching 0 and start the new transition

@KaelWD
Copy link
Member

KaelWD commented Oct 14, 2021

This is solved in v3 by reversing the current transform before calculating the position: https://github.com/vuetifyjs/vuetify/blob/3940216aa43656d4d6e3189826e6bcef4495fbeb/packages/vuetify/src/util/animation.ts#L8-L7

@glen-84 glen-84 added C: VMenu VMenu T: bug Functionality that does not work as intended/expected labels Oct 24, 2021
@KaelWD KaelWD changed the base branch from master to v2-stable February 5, 2023 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VMenu VMenu T: bug Functionality that does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] v-menu position is glitched when trying to open animation before close animation completes
4 participants