visionmedia / page.js

Micro client-side router inspired by the Express router
http://visionmedia.github.com/page.js
7.67k stars 687 forks source link

Fix for anchors in shadow dom #475

Closed mgibas closed 6 years ago

mgibas commented 6 years ago

In shadow dom we can not traverse path using parentNode cause there can be document-element in between other elements. Once document-element is found it will return from onclick (parentNode is undefined) and reload the page.

Why path array is build that way? No idea. Seems to be expected though cause for loop is also used in iron-location click handler - they should know a bit more about shadoDom :)

https://github.com/PolymerElements/iron-location/blob/2486d275bef819aa3eedffee69158f695580d40f/iron-location.html#L342

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.0%) to 90.449% when pulling 5648ef91a076d4caf20c54287082c2507ca5ff6a on mgibas:master into 73fefebc3ad425f16c42ed642de5312a5bc8d115 on visionmedia:master.

cecilia-sanare commented 6 years ago

Just another temporary workaround until this change and #476 are resolved.

// TODO: Remove this once https://github.com/visionmedia/page.js/pull/475 is merged and deployed!
// Personally wouldn't throw this logic into production as it 
// overrides the builtin path implementation in Chrome.
// However it's a decent workaround for development purposes.
// Throw this before any page.js logic is invoked.
Object.defineProperty(Event.prototype, 'path', {
  get: function() {
    var path = this.composedPath && this.composedPath() || [this.target];
    var anchor = null;
    for (var i = 0; i < path.length; i++) {
      var element = path[i];
      if (element.tagName === 'A' && element.href) {
        anchor = element;
        break;
      }
    }
    return [anchor];
  }
});
mgibas commented 6 years ago

@matthewp - will this be considered for merge ? I would take care of code coverage then

matthewp commented 6 years ago

Yep!