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.

Sign up for GitHub

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

Jump to bottom

interceptEvents doesnt catch shadowDom links events #14

Closed
2 tasks done
ar2r13 opened this issue Apr 2, 2023 · 2 comments · Fixed by #17
Closed
2 tasks done

interceptEvents doesnt catch shadowDom links events #14

ar2r13 opened this issue Apr 2, 2023 · 2 comments · Fixed by #17

Comments

@ar2r13
Copy link

ar2r13 commented Apr 2, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure it has not already been reported

Version (i.e. v2.x.x)

1.0.1-alpha.192

Node.js version (i.e. 18.x, or N/A)

No response

Operating system

None

Operating system version (i.e. 20.04, 11.3, 10, or N/A)

No response

Description

interceptEvents doesnt catch shadowDom links events.

function interceptWindowClicks() checks only parents of event target. I guess it would be better to check is event path contains any a[href]

Steps to Reproduce

import('@virtualstate/navigation').then(({ applyPolyfill }) => applyPolyfill({
    interceptEvents: true
}))

### Expected Behavior

_No response_
@fabiancook
Copy link
Contributor

fabiancook commented Apr 27, 2023 •
edited
Loading

Thanks I will look into this specific case!

Do you have a static html page that can trigger the issue? If you can make a PR, starting from

navigation/example/index.html

Lines 1 to 84 in 17132f9

<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Polyfill Example</title>
</head>
<body>
<script src="polyfill-rollup.js"></script>
<script>
window.addEventListener("load", loaded)
function loaded() {
console.log("Initial History State", history.state);
console.log(navigation)
const urlSpan = document.getElementById("url");
const app = document.getElementById("app");
navigation.addEventListener(
"navigate",
event => {
console.log("Navigate event", event)
const url = new URL(event.destination.url);
console.log(event.destination.url);
console.log("Submitted Name:", event.formData?.get("name"));
console.log("Previous State:", navigation.currentEntry.getState());
console.log("State:", event.destination.getState());
console.log("Same Document:", event.destination.sameDocument);
urlSpan.innerText = url.pathname;
app.innerHTML = "";
if (url.pathname === "/some/other") {
console.log("Preventing default");
event.preventDefault();
try {
console.log("Navigated");
navigation.navigate("/", {
state: "returned"
});
} catch (error) {
console.error(error);
}
} else if (url.pathname === "/some/page") {
event.intercept(() => {
app.innerHTML = `
<form action="/" method="post">
<input type="text" name="name" value="" placeholder="Name" />
<button type="submit">Submit</button>
</form>
`;
})
} else {
event.intercept()
}
}
);
if (navigation.currentEntry) {
urlSpan.innerText = new URL(navigation.currentEntry.url).pathname;
}
console.log(navigation.currentEntry.url)
}
</script>
<p>Current Url <span id="url"></span></p>
<a href="/">Home Link</a>
<a href="/some/page">Some Page Link</a>
<a href="/some/other">Some Other Page Link</a>
<a href="/esm.html">ESM Link</a>
<div id="app"></div>
</body>
</html>

This page should be loading and then using the polyfill in a static way. The polyfill files thats referenced were are built files that I am just pushing to git... but you can update them too by building the project again.

I was using npx http-server -p 3000 ./example to serve the standalone example files for debugging. I should really add this as a script now too.

I will then be able to load this up in playwright and trigger the behaviour, and make a test around it like this, and will also be able to load it up directly in a browser and see how the behaviour works. Cheers!

(Sorry for the late reply I have been on holiday for the last month so haven't seen any github notifications!)

@aearly aearly mentioned this issue Sep 1, 2023
Merged
4 tasks
@fabiancook
Copy link
Contributor

fabiancook commented Sep 2, 2023 •
edited
Loading

With the changes from #17, this should be good to go from here. I tested this in safari, I'll add some playwright tests to match though in the near future.

Some non important but informational bits....

In the image below the target of the originalEvent is <link-element>, an element with a shadow dom:

class LinkElement extends HTMLElement {
  constructor() {
    super();
    const shadow = this.attachShadow({ mode: "open" });
    shadow.innerHTML = '<a href="/">Link within Shadow DOM</a>';
  }
}

window.customElements.define('link-element', LinkElement)

image

Using composedPath()

image

If we inspect targets we can see the inner anchor element being accessible.

function getComposedPathTarget(event) {
    if (!event.composedPath) {
        return event.target;
    }
    const targets = event.composedPath();
    return targets[0] ?? event.target;
}

image

This only accounts for shadow DOMs using open mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow intercepting events from within shadow DOM. aearly/navigation
2 participants
@fabiancook @ar2r13