Skip to content

click outside case. Microtasks and addEventListener #9464

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

Closed
Gormartsen opened this issue Feb 8, 2019 · 13 comments
Closed

click outside case. Microtasks and addEventListener #9464

Gormartsen opened this issue Feb 8, 2019 · 13 comments

Comments

@Gormartsen
Copy link

What problem does this feature solve?

Ability to call method when click is outside element(component)

In 2.5.x there was simple way to do via document.addEventListener and el.contains(event.target)

It was useful specially via directive like v-click-outside="componentMethod"

After inspecting live circle in 2.6.x I found that eventListener happen after click processed inside component and event.target.parentElement is null (in case if there is inside click process that change component display)

Let me peripheries it.
In case if we have a component that change it's own rendering and we listen for EventListener click on $el of the component - EventListener function receive event.target of the removed DOM element.

Going back to the root issue.
How we can simply monitor multiple elements that need to change it's own display if user clicked outside element. Like dropdown element for example.

What does the proposed API look like?

Have no idea.

@Gormartsen
Copy link
Author

Gormartsen commented Feb 9, 2019

Here is examples:

Working 2.5.22 https://jsfiddle.net/gormartsen/uz18a0dx/23/

Not working 2.6.3 https://jsfiddle.net/gormartsen/Lrae3jbg/3/

Basically clickOutside is happening after clickInside processed as a result e.target.parentElement is null and cannot be identified as $el.contains

Any suggestions how it can be done for 2.6.x

@yyx990803
Copy link
Member

Just add @click.stop on your component? https://jsfiddle.net/yyx990803/uefngc30/

@Gormartsen
Copy link
Author

Gormartsen commented Feb 9, 2019

@yyx990803 Hi Evan.
Please click first block button, then second.

Expected rollback to click me button in first block when click in second block inside.

@Gormartsen
Copy link
Author

Gormartsen commented Feb 9, 2019

PS I understand that you are busy guy and it's sometimes annoying to respond "stupid" question.

Trust me before write down here, I did alot of research (include new code reading and debuging) and this example is very simplified concept of more complicated Vue APP that has multiple components that need to change state if click happen inside another component.

If you like, I can expand example to more complicated, but please show some desire to communicate.

@Gormartsen
Copy link
Author

Also, your example has issue on iOS 10.2.1 after click button, clicking outside is not rolling back state to extended false, so click outside is not working at all.

@yyx990803
Copy link
Member

Oh I didn't notice there's another component. In that case... just attach your document listener in capture mode.

@Justineo
Copy link
Member

Justineo commented Feb 9, 2019

You'd better set the useCapture option to true while attaching the document-level event handler because it prevents the event propagation from being accidentally stopped by other event handlers. And in this case it also fixes your issue.

@Gormartsen
Copy link
Author

@yyx990803 Yep. It works now.
@Justineo tnx for hint.

Future visitors, working example is here: https://jsfiddle.net/gormartsen/Lrae3jbg/5/

@yyx990803 With this code there is another issue - iOS 10.2.1 get stuck with "expanded" state and do not react on "click outside"

Do you need it filed as separated issue?

How to reproduce:
Open on mobile link https://jsfiddle.net/gormartsen/Lrae3jbg/5/show

click first button, click second button (so far so good) but then clicking on button produce only click outside but not inside. So @click on button is not get fired.

On iOS simulator 11.4 - issue exists but in different state. Clicking between components or around them - does not produce clickoutside.

on safari 12.0.3 - all is good with onCapture.

As far as I understand it can be connected with isUsingMicroTask == true

@Gormartsen
Copy link
Author

Gormartsen commented Feb 9, 2019

@yyx990803 I get puzzled how come it started to work in such strange way.

So I did more debugging and difference is in lifecycle:

useCapture = false

  • process inline click (set extended == true)
  • re-render component because of v-if="extended"
  • process eventHandler, where event.target is already removed DOM element so everyone receive outside call

useCapture = true

  • process eventHandler, where event.target is still exists
  • process outside click after compare element.target via element.contains for each component
  • process inline click (set extended == true)
  • re-render component because of v-if="extended"

I did tests only in Safari 12.0.3. I don't know if behaviour is different in other browsers.

useCapture = true and iOS works unstable, so there is no way to relate to this code.

@Gormartsen
Copy link
Author

Also there is another "unexpected behaviour"

See: https://jsfiddle.net/gormartsen/Lrae3jbg/13/

lifecycle:

  • click button "click me"
  • clickInside processed. expended become true
  • rendered expanded v-if
  • revert click processed (attached via @click to root element of component)

WHat is expecting:

  • revert click processing first (top level)
  • clickInside processed second (one level down)

What is happening:

  • clickInside processed first
  • clickRevert processing next

@yyx990803
Copy link
Member

yyx990803 commented Feb 9, 2019

This is not a support thread. If you have an isolated bug, open a separate issue with a minimal reproduction, showing the bug and the bug only. The behavior has a lot to do with how you decided to implement your logic using a mix of Vue and direct DOM manipulations, so it's no longer a "bug" per se.

@Gormartsen
Copy link
Author

@yyx990803 Sorry to see tension here.
Opensource is about share and communication, and I do my best to give back with what ever I can help with.

I am not looking for support here. I am bringing a feedback to you.

I started to dig dipper and as you can see in previous comment - it's not about direct DOM manipulation but revert event processing.

Please review it again.

I hope you just have a bad day and it's not your usual behaviour.

@yyx990803
Copy link
Member

We have limited bandwidth, so we require bug reports to be clear, concise and exhibit what exactly Vue is doing wrong. This is about respecting maintainer's time.

Your original case does not qualify as a bug, because it involves your custom directive which has specific expectations on DOM state (contains check). This is not fundamentally caused by Vue, because you will have the same problem if your write vanilla js that synchronously removes the button that triggered the click before the event propagates to the document.

For your revert "unexpected behavior" - note that normal mode listeners captures events bottom up, so inner being fired first is the expected behavior. If you want your revert to fire first, then it must be in capture mode. Again this is not even a problem in Vue - so we end up just helping you dig through code to figure out how to solve your problem - exactly what a support session is.

The iOS 10 issue is probably a real problem, and you should file a separate issue with a boiled down reproduction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants