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

Actions menu / Popover flying arrow on close #1610

Closed
PVince81 opened this issue Nov 26, 2020 · 8 comments
Closed

Actions menu / Popover flying arrow on close #1610

PVince81 opened this issue Nov 26, 2020 · 8 comments
Labels
bug Something isn't working

Comments

@PVince81
Copy link
Contributor

To be able to see it, need the actions popover to overlay some dark background.

Steps

  1. Open the talk app
  2. Start a call (no other participants needed)
  3. Click the three dots on the top right and focus your eyes on the arrow
  4. Click outside to close the actions popover
  5. Repeat the last two steps and look for the arrow

Expected result

The closing animation of the menu doesn't leave a flying arrow.

Actual result

The arrow will move to some direction and keep its opacity until the menu is closed.

With the arrow, I mean this one:
image

popover-arrow-animation

@PVince81 PVince81 added the bug Something isn't working label Nov 26, 2020
@PVince81 PVince81 reopened this Jan 26, 2021
@PVince81
Copy link
Contributor Author

PVince81 commented Jan 26, 2021

It as only solved by #831 by pure luck. (and reverted)

I found the root cause: during the closing animation of the Actions menu, the property "opened" is already set to false, and the menu contents DOM is cleared too early. This is what prompts popper to update the position during the animation.

A quick fix is to remove the "opened" condition and always let the menu render itself, even when not opened.
Another idea would be to add an extra flag to disable rendering that toggles after a delay. But not sure if such flag is worth the hassle...

@PVince81
Copy link
Contributor Author

quick patch here:

diff --git a/src/components/Actions/Actions.vue b/src/components/Actions/Actions.vue
index a586b75..f88e23b 100644
--- a/src/components/Actions/Actions.vue
+++ b/src/components/Actions/Actions.vue
@@ -140,7 +140,7 @@ https://www.w3.org/TR/wai-aria-practices/examples/menu-button/menu-button-action
                        </button>
 
                        <!-- Menu content -->
-                       <div v-show="opened"
+                       <div
                                ref="menu"
                                :class="{ open: opened }"
                                tabindex="-1"
@@ -154,7 +154,7 @@ https://www.w3.org/TR/wai-aria-practices/examples/menu-button/menu-button-action
                                @mousemove="onMouseFocusAction">
                                <!-- menu content -->
                                <ul :id="randomId" tabindex="-1">
-                                       <template v-if="opened">
+                                       <template>
                                                <slot />
                                        </template>
                                </ul>

but I now still have mixed feelings... if Actions menus are used in a list, it might cause too many DOM elements to be rendered.

In general I'd rather expect the VPopover/v-tooltip/popper to destroy its contents after hiding.

@PVince81 PVince81 self-assigned this Jan 26, 2021
@PVince81
Copy link
Contributor Author

PVince81 commented Mar 9, 2021

to clarify, Popper (the v-tooltip and Popover backend) has a resize observer that checks if the popup contents changes

whenever we flip the "opened" state, it will render or unrender the elements inside the popup, which is detected as change, so popper will update the popup's position

now, since we also have some animation in place, it will animate to the new position

@raimund-schluessler
Copy link
Contributor

@PVince81 Is this still an issue?

@PVince81
Copy link
Contributor Author

yes, reproducible on c.nc.com (NC 24.0.4rc1)

image

this time it's on this menu, if you open and close multiple times, you can see the arrow flying to the right

once Talk is updated to floating-vue (are we using it for popovers now?) we can try again

@PVince81
Copy link
Contributor Author

I tried again, I needed to click 10-15 times with different speeds to make it appear.

Now, am not sure if it's worth fixing... The only thing that matters I think is that we move away from v-tooltip for the Popover, as v-tooltip has many issues with timings and events.

@raimund-schluessler
Copy link
Contributor

We use floating-vue now for the popovers. @PVince81 Could you check whether this still is an issue for you, please?

@susnux
Copy link
Contributor

susnux commented Nov 11, 2023

Should be fixed with using floating-vue, if you still experience the issue please reopen.

@susnux susnux closed this as not planned Won't fix, can't repro, duplicate, stale Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants