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

virtual-tour-plugin new arrows not working in web-components #1412

Closed
sho-ax opened this issue Aug 21, 2024 · 8 comments
Closed

virtual-tour-plugin new arrows not working in web-components #1412

sho-ax opened this issue Aug 21, 2024 · 8 comments
Labels
Milestone

Comments

@sho-ax
Copy link

sho-ax commented Aug 21, 2024

Describe the bug

I'm using the Photo Sphere Viewer with the Virtual Tour Plugin in lit web-components. Since the new arrows in the Virtual Tour Plugin are implemented, they are in my case not clickable anymore while the hover effects still work. I discovered that the "mouseup" eventlistener in core/index.module.js in line 4782 is used for the click on an arrow to move to another pano. My problem is: since the eventlistener is added to window, the returned target of the event is the web-component, in which the Photo Sphere Viewer is placed.

In my case it solved the issue, when I changed the current code in core/index.module.js in the class EventsHandler in function init():

...
4781 window.addEventListener("mousemove", this, { passive: false });
4782 window.addEventListener("mouseup", this);
4783 this.viewer.container.addEventListener("touchstart", this, { passive: false });
...

to the following code:

...
4781 window.addEventListener("mousemove", this, { passive: false });
4782 this.viewer.container.addEventListener("mouseup", this);
4783 this.viewer.container.addEventListener("touchstart", this, { passive: false });
...

Online demo URL

No response

Photo Sphere Viewer version

5.9.0

Plugins loaded

virtual-tour-plugin; gallery-plugin;

OS & browser

MacOS: Firefox

Additional context

No response

@sho-ax sho-ax added the bug label Aug 21, 2024
@mistic100
Copy link
Owner

binding the mouseup to the container is not a suitable solution because it leads to unvoluntary movement when the user moves the cursor away from the viewer before releasing the button.

Please provide a minimal reproduction, I don't know what lit is.

@mistic100 mistic100 added the awaiting answer Additional information is required. Will be closed after 14 days. label Aug 21, 2024
Copy link

This issue has been automatically marked as stale because not enough information was provided. It will be closed if no further activity occurs.

@github-actions github-actions bot added the Stale label Aug 29, 2024
@sho-ax
Copy link
Author

sho-ax commented Sep 1, 2024

Here is a Demo with the PSV in a WebComponent in form of a LitElement: https://codesandbox.io/p/sandbox/virtual-tour-with-lit-hx3c2j

And a more pecise description of the problem:

WebComponents

WebComponents are a technology to build customElements, custom encapsulated HTML Elements with internal encapsulated html and js code. These customElements can be used as default html Tags/Elements in the HTML DOM tree. This concept has many advantages in bigger, complex WebApps but often causes problems with classical JS libraries.
LitElements are just an easier to use framework around WebComponents and have nothing directliy to do with this Problem.

Detailed description of this specific Problem

CustomElements also encapsulate events, which means that if we catch an event outside a customElement that is dispatched inside, the event target is the outer customElement and not the original element, in this case the arrow button svg, which is defined inside the customElement.
Instead, there is a method event.composedPath() that returns a list with all Elements of the whole path of the event trough the dom tree. Hence, it also includes the clicked arrow div Element.
You can see this with the mouseup event in the handleEvent method of the PSV EventHandler, when you have clicked on an link arrow in the virtual tour.

Possible solution

So, a possible solution could be to infer the clicked arrow element from the composedPath instead of the event target Element. The composedPath should be available in all modern browsers.

@github-actions github-actions bot removed the Stale label Sep 2, 2024
@mistic100 mistic100 removed the awaiting answer Additional information is required. Will be closed after 14 days. label Sep 3, 2024
@mistic100
Copy link
Owner

When using closed shadow dom and the event listener is attached to window, the composedPath stops at the web component element, the rest of the hierarchy is "lost".

If I attach the listener to viewer.container.getRootNode() it exhibits the same buggy behaviour as described in my first message (when using a Web Component, else it works as before).

I can listen to "mouseout" on the container to stop movements, but that only works on devices with a mouse, I also need to listen for touchmoves outside the viewer.

If I listen on both window and the container, the handler is called twice (obviously) without any way to know which one is which (when not using web components, if using web components I can filter out the one where the viewer container is not in the composed path).

What a mess...

@sho-ax
Copy link
Author

sho-ax commented Sep 6, 2024

Lit uses open shadow dom as default, so in my case this would work fine for me either way. I don't know if there would be a more elegant solution for using closed shadow dom but if you would like it to work in as many cases as posible, i think your consideration of using both a listener on window and the container would work anyway. You can check the composed path regardless of whether using web components or not. In my quick test the evt.composedPath() worked also without a web component arround the photo-sphere-viewer and deliveres the container when the mouse button is lifted inside the container but not, when it was lifted outside.

@mistic100 mistic100 added this to the 5.9.1 milestone Sep 7, 2024
@mistic100
Copy link
Owner

I used composedPath everywhere I could and also added a check to log an error if a closed shadow DOM is used.

@sho-ax
Copy link
Author

sho-ax commented Sep 7, 2024

thank you very much for your quick reaction!

Copy link

github-actions bot commented Sep 7, 2024

This feature/bug fix has been released in version 5.10.0.

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

No branches or pull requests

2 participants