Template: remove "base href" and fix javascript link-fixing #10549
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
opening as draft, as we first need the links fixed; #10548 |
thaJeztah
force-pushed
the
remove_base_href
branch
from
5ed1ca7 to
e545869
Compare
thaJeztah
changed the title
thaJeztah
force-pushed
the
remove_base_href
branch
from
e545869 to
d43c152
Compare
thaJeztah
changed the title
Member
Author
|
Temporarily moving back to "WIP", as I still see issues occurring of incorrect links being generated; let me investigate if there's a configuration that I can change |
thaJeztah
force-pushed
the
remove_base_href
branch
from
d43c152 to
99f82df
Compare
This was referenced Apr 10, 2020
We cannot use relative links in includes, because: - The jekyll-relative-links, is not called on includes, so markdown-links are rendered as-is. - These files are included in various locations on the website; because of that, it's not possible to compose a relative link to other Markdown files, so we're falling back to using absolute URLs, relative to the root of the website. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- remove some redundant whitespace - use "remove_first" instead of "replace", because the strings to replace should only occur once. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Removing this "check" because it made the template difficult to read, and duilding the docs won't fail if these values are missing. If a page is empty, it's probably fast to find why anyway. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When generating HTML pages, Jekyll will generate a directory named after the name of the Markdown file, and generate an index.html file inside that directory. For example, for a markdown file named /foo/bar/mypage.md, Jekyll generates a HTML file named /foo/bar/mypage/index.html. This means that all links relative to mypage.md, and expect those links to be relative to the /foo/bar/ directory, will actually end up being relative to /foo/bar/mypage/. Unfortunately, Jekyll / Liquid does not have a variable that holds the parent directory of the _markdown_ file, so we have to generate it by taking `page.path` (which holds the absolute path of the markdownfile), and remove the filename from that path. After generating that path, we prepend that path to URLs linking to related commands (parent commands and child commands), as all reference files are in the same path. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
remove_base_href
branch
from
99f82df to
b21b874
Compare
thaJeztah
changed the title
Member
Author
|
@usha-mandya @StefanScherer @silvin-lubecki I separated this from #10615 again; this should be ready for review, and fixes #10647 |
|
Deploy preview for docsdocker ready! Built with commit 342660f |
- remove some stray empty lines - put liquid code that was before the opening HTML inside a HTML comment, to prevent IDE's from marking it as invalid HTML - fix some indentation - fix some minor linting issues Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The base href caused browsers to produce incorrect URLs for
anchor links on various pages, for example, pointing to:
https://docs-test.docker.com/engine/reference/#foreground
Instead of
https://docs-test.docker.com/engine/reference/run/#foreground
Also cleaning up and fixing the JavaScript workaround for links
in include-files;
- only fix up links in the main content, not in other parts
of the page
- don't fix up anchor links, absolute links, or links that don't
contain `.md`: for those we can assume they were generated
correctly, and if not, those are links that should be fixed in
the markdown source, not fixed afterwards.
- document the function for future readers.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The JavaScript "link fix" looks to be only needed for pages where Markdown is included, and which contain relative links. Now that we modified all local includes to use absolute links, the only location where links are not properly generated, is in the reference documentation. If broken links are found elsewhere in the website, those links are legitimately broken, and should be fixed in the markdown source, not fixed-up afterwards. This patch moves the javascript to the cli.md include, so that the script is only run on the reference pages instead of on every page. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Now that we no longer have a baseURL, we can simplify the generated links to just contain the anchor. This also provents issues if the visited page already has an anchor set. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
remove_base_href
branch
from
b21b874 to
342660f
Compare
StefanScherer
approved these changes
Apr 23, 2020
Member
StefanScherer
left a comment
StefanScherer
left a comment
There was a problem hiding this comment.
SGTM
I tested a bit in the /docker-for-windows/troubleshoot page and I couldn't find a problem.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The base href caused browsers to produce incorrect URLs for anchor links on various pages, for example, pointing to:
Instead of
While there might be links generated incorrectly (links including
.md), we should fix those when generating the HTML instead of trying to fix them up using JavaScript.Unfortunately, Jekyll doesn't process links to markdown files in includes, so for includes in this repository, these were changed to "absolute" "html" links (not linking to Markdown). After that, the only place we need the JavaScript link fix is within the reference docs.
I rewrite the script to be more efficient, documented each step (sorry, quite verbose on the comments), and limited its scope to only run on the reference docs, and on those, only on links within the "content".
fixes #2655
fixes #10647
relates to #247
relates to #8070
relates to #6772
relates to #5711
relates to #5388
relates to #3952
relates to #2655