Skip to content

when using relativePath: true, links on the sidebar are always relative to the current page #1139

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
1 task done
dellagustin opened this issue Apr 26, 2020 · 12 comments
Closed
1 task done

Comments

@dellagustin
Copy link

Bug Report

this is a duplicate of #925, that was closed automatically.

Steps to reproduce

I have prepared an example here: https://dellagustin-sap.github.io/docsify-relative-path-bug-report/#/

Click first on Subpage1 - Absolute (on the sidebar), then on Subpage1 - Relative

What is current behaviour

A 404 page is shown

What is the expected behaviour

Subpage 1 should be shown

Other relevant information

  • Bug does still occur when all/other plugins are disabled?

  • Your OS: Windows 10 home

  • Node.js version: N/A

  • npm/yarn version: N/A

  • Browser version: Google Chrome 80.0.3987.163

  • Docsify version: 4.11.3

  • Docsify plugins: none

Please create a reproducible sandbox

Edit 307qqv236

https://dellagustin-sap.github.io/docsify-relative-path-bug-report/#/

Mention the docsify version in which this bug was not present (if any)

@dellagustin
Copy link
Author

dellagustin commented Apr 26, 2020

I have considered whether this behavior was intended to cover some specific scenario, but could not think of any in which it would be beneficial.

I have browsed the code, the link rendered uses the current route to resolve relative links:

href = router.toURL(href, null, router.getCurrentPath());

It has no access to current file for the sidebar (I suppose this is also true for the navbar).
The only place where this information is available seems to be:

function loadNested(path, qs, file, next, vm, first) {

I could not think of an easy solution.
There are many layers between where the information is available and where it is used.

I have considered the following:

  1. Modiy loadNested also pass down the current path to the next callback (maybe as a 3rd parameter, or enriching the section parameter, as it is a dictionary.
  2. Pass the current path to _renderSidebar and _renderNav
  3. Extend the HashHistory router to return the current path (of sidebar or navbar) on getCurrentPath
  4. Instatiate a different compiler to use for redering sidebar and navbar, that would use as router the new extended HashHistory

I did not check yet how to create automated tests for this.

@anikethsaha
Copy link
Member

Thanks for the detailed issue. 💯

If I am not wrong, when you are in subpage1 (absolute) , you click on the subpage1 (relative) and its searching for the page in that directory instead of from root . right ?

if this is, then its the intended behavior as sidebar is always at root (of the current folder) so the base path will be the current path. if you want to go up one directory in order to render that page you need to give related path from where the sidebar is.

@dellagustin
Copy link
Author

Hi @anikethsaha

If I am not wrong, when you are in subpage1 (absolute) , you click on the subpage1 (relative) and its searching for the page in that directory instead of from root . right ?

Right, if I understood that correctly.

To give more details:
In my example, if you are at https://dellagustin-sap.github.io/docsify-relative-path-bug-report/#/subpages/subpage1 (route is /subpages/subpage1, you will get here from / by clicking either the absolute or relative link), and then you click in the relative link, it will link to https://dellagustin-sap.github.io/docsify-relative-path-bug-report/#/subpages/subpages/subpage1

Now, in my example the sidebar markdown file is always at root (not of the current folder, but https://dellagustin-sap.github.io/docsify-relative-path-bug-report/sidebar.md) - and that is why I think that the links on the sidebar should be resolved relative to where the sidebar file was found.

About:

as sidebar is always at root (of the current folder)

I am not sure what you mean with always here, because loadNested will recursively search for the sidebar up the path structure: https://docsify.js.org/#/more-pages?id=nested-sidebars

If the current directory doesn't have _sidebar.md, it will fall back to the parent directory.

@anikethsaha
Copy link
Member

as sidebar is always at root (of the current folder)

loadNested ? did you mean loadSidebar

I am not sure what you mean with always here, because loadNested will recursively search for the sidebar up the path structure: https://docsify.js.org/#/more-pages?id=nested-sidebars

sorry, I thought you had the aliases setup to fallback to root.

this needs investigation. I will check once again.

cc @docsifyjs/reviewers , thoughts ?

@dellagustin
Copy link
Author

dellagustin commented Apr 26, 2020

loadNested ? did you mean loadSidebar

No actually meant this function:

function loadNested(path, qs, file, next, vm, first) {

@anikethsaha
Copy link
Member

ok cool, I will check that soon for sure 👍 .

@stale
Copy link

stale bot commented Jun 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 25, 2020
@CxRes
Copy link

CxRes commented Aug 4, 2020

I think I can solve this issue. My strategy is to pass a isParent property to the next function in loadNested if it is invoked a second time which is passed to _renderSidebar and then compiler.sidebar. Now ideally I should be able to ask the compiler to append '../' to each link or I can mutate paths from _sidebar.md to append '../'.

  function loadNested(path, qs, file, next, vm, first) {
    path = first ? path : path.replace(/\/$/, '');
    path = getParentPath(path);

    if (!path) {
      return;
    }

    get(
      vm.router.getFile(path + file) + qs,
      false,
      vm.config.requestHeaders
    ).then(
      next,
      function (_) { return loadNested(path, qs, file, (r) => next(r, true), vm); });
  }

  ...

        var fn = function (result, isParent) {
          this$1._renderSidebar(result, isParent);
          cb();
        };

        // Load sidebar
        loadNested(path, qs, loadSidebar, fn, this$1, true);
      
  proto._renderSidebar = function(text, isParent) {
     ...
     this._renderTo('.sidebar-nav', that.compiler.sidebar(result, maxLevel, isParent));
     ...
  }

Compiler.prototype.sidebar = function sidebar (text, level, isParent) {
  ...
  if (text) {
      html = this.compile(text, isParent && {baseUrl: '../'}); // using this to modify marked.setOptions
  }
  ...

An alternative could be to pass the path from loadNested and use it to build absolute urls before passing it to the compiler.

But I have a couple of questions:

  1. How do I ask marked to append ../ to links. Adding baseUrl does not seem to work.
  2. How do I get a handle to relativePath in compiler.sidebar.

@sy-records
Copy link
Member

I tried it and I don't think it's a problem.

no get 404

image

The two files in your sidebar should both use absolute paths.

- A section
    - [Subpage1](/subpages/subpage1.md) - Absolute
    - [Subpage1](/subpages/subpage1.md) - Relative

@MrBlenny
Copy link

MrBlenny commented Sep 2, 2021

I was having this issue but was able to resolve it for now by writing the _sidebar.md in html instead of markdown.

i.e.

    - [Subpage1](/subpages/subpage1.md)
    - [Subpage2](/subpages/subpage2.md)

becomes:

    - <a href="#subpages/subpage1">Subpage1</a>
    - <a href="#subpages/subpage2">Subpage1</a>

note, # is added and .md is removed.

@brandones
Copy link

I'm having this issue as well. Another workaround is to make the paths in _sidebar.md absolute. The problem is that then the file is not usable independently of docsify; e.g. if you view it in GitHub the links will not resolve correctly. Also you can't use markdown-link-check on the file for the same reason—this is perhaps a bigger problem. I maintain a substantial piece of documentation and rely on markdown-link-check to keep it working.

@trusktr
Copy link
Member

trusktr commented Jan 12, 2022

I too have been bitten by this. This will be officially solved in #1684 with a new option (for backwards compatibility, so that existing options like relativePath work the same).

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

No branches or pull requests

7 participants