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

Using data-navigo attribute on a website without a server leads to absolute file path beeing used as location #120

Closed
JonasTaulien opened this issue Apr 25, 2017 · 6 comments

Comments

@JonasTaulien
Copy link

JonasTaulien commented Apr 25, 2017

Hey :)
I'm really sorry, that the content of this Issue was just 'Hey :)' in a previous version. I accidentally send this request hitting 'Ctrl + Enter' and then left for lunch before I was able to finish writing this Issue.

Context:

I want to use Navigo for an offline website, more precise: A documentation for a tool I'm building. The documentation will be shipped in the code for the tool. So when someone needs help with the tool, one only has to open 'file:///var/users-project/vendor/the-tool/documentation/index.html' in his browser.

Steps to reproduce:

I created the index.html which contains links of the form:

<a href="overview/getting-started" data-navigo>Getting Started</a>

and defined the router like this:

var router = new Navigo(null, true, '#');
router.on({
    ':article': function (params) {
        var article = params.arcticle;
        //Here I want to replace the content of the body with the content of the file  '{{ article }}.html'
    }
}).resolve();

By doing this, I just have to add new article-html-files and links to them, but don't have to touch the router configuration when adding an article to the documentation.

When I now open the page in a browser with the link file:///var/the-tool/documentation/index.html and click on the Getting Started link, the URL will be file:///var/the-tool/documentation/index.html#/var/the-tool/documentation/getting-started instead of expected file:///var/the-tool/documentation/index.html#overview/getting-started.
Also the article variable gets the value getting-started instead of expected overview/getting-started.

Possible fix

Without having an overview of the inner workings of Navigo. I suggest to change the code from

// ...
Navigo.prototype = {
    //...
    updatePageLinks: function updatePageLinks() {
        // ...
        this._findLinks().forEach(function (link) {
            // ...
            var location = link.pathname;
// ...

to

// ...
Navigo.prototype = {
    //...
    updatePageLinks: function updatePageLinks() {
        // ...
        this._findLinks().forEach(function (link) {
            // ...
            var location = link.getAttribute('href');
// ...

This way the url successfully will be changed to file:///var/the-tool/documentation/index.html#overview/getting-started.
But, the article variable still has getting-started instead of expected overview/getting-started as value. I currently don't know how to fix that.

Am I using Navigo incorrectly or is this a valid bug, that (when fixed) would not break any existing code, that uses Navigo?

Thanks :)

@krasimir
Copy link
Owner

Sorry, what you mean without a server? If Navigo works in a mode where we use the History API then you need to make some back-end changes to get the things working.

@JonasTaulien
Copy link
Author

JonasTaulien commented Apr 25, 2017

I updated my question and again: I'm sorry, that I submitted this issue before writing the content. I am not sure if you will be notified about the update, but now you definitely will.

@JonasTaulien
Copy link
Author

JonasTaulien commented Apr 25, 2017

When I change the router configuration to

var router = new Navigo(null, true, '#');

router.on(/#(.*)/, function (article) {
    console.log('Article is: ' + article);
});

router.resolve();

and apply the mentioned fix, then Article is: overview/getting-started gets logged.
That means, when the mentioned fix would get applied, everything else is fine :)

@krasimir
Copy link
Owner

@jonasrudolph I'll expose that reading of the URL as a api method. It was before using getAttribute('href') but we changed it to .pathname so we fix another bug. So in order to satisfy the other issue I'll keep pathname as default but will give an option to use getAttribute.

krasimir pushed a commit that referenced this issue Apr 25, 2017
@krasimir
Copy link
Owner

4.7.0 released. The router now contains a public methodgetLinkPath. You may overwrite it like so:

router.getLinkPath = function (link) {
  return link.getAttribute('href');
};

Here's a test case for reference https://github.com/krasimir/navigo/blob/master/test/spec/InBrowser.spec.js#L355-L379

Let me know if it works as expected.

@JonasTaulien
Copy link
Author

Wow, that was fast!
I really like your usage of the strategy pattern here :)
I had a look at the implementation and the test - And I don't know why it would not work as expected.

Thank you really much, Krasimir, for taking the time to resolve this issue!

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

2 participants