Fix nav URL rebasing to work across pages at different depths#206
Fix nav URL rebasing to work across pages at different depths#206gpx1000 wants to merge 1 commit intoKhronosGroup:mainfrom
Conversation
Rewrites captured nav hrefs/src to be component-root-relative instead of page-relative. The shared nav loader now accepts a per-page prefix that adjusts URLs back to each page's URL space, preventing duplication bugs like `/source/source/` when nested pages reference the same shared nav.
|
Preview site published: https://KhronosGroup.github.io/Vulkan-Site/PR-206/ |
|
Okay that looks fixed. NB: due to the issues of how we make the PR pages work this is the way to test it: Technical reason for why this is necessary: The way the PR pages are made small is they reuse sites that have not changed that are served in the base main branch. A correllary of that effect is you need have PR-206 in your address bar when the site is loaded so that it resolves the javascript that is fixed rather than the broken code in main. Advantage of this work around issue, you can validate after navigating to another page that is served on the main site, you can validate that it is broken on main then hitting the back button to get to the PR preview site that same link will resolve correctly. |
|
The review page linked by the github bot above still has the issue. Or is there another easy way to test? |
|
Could you give me repo steps? |
|
With above link (https://KhronosGroup.github.io/Vulkan-Site/PR-206/) navigate to e.g. https://github.khronos.org/Vulkan-Site/refpages/latest/refpages/source/VK_VERSION_1_4.html Links in nav will have a duplicate "source" added to their url afterwards. I've attached a video: brave_EeyffCYblw.mp4 |
|
EDIT: cancel comment, I was looking at the wrong zipfile contents. The latest CI artifact does not seem to have this problem. ---- old comment ---- I pulled the latest artifacts from the PR and viewed locally to reproduce the publication environment, rather than dealing with how the GH preview pages are setup. Can confirm that the nav index is still broken. Starting at refpages/latest/refpages/index.html I can navigate to arbitrary refpages from the sidebar, but from such a new refpage, the nav index is then broken with the 'source/source/' showing up again. |
|
Thanks, working on finding this now. |
Sorry, I withdraw the comment. Was looking at the wrong artifact contents. |
|
Phew, I was looking for the bug and couldn't find it |
|
@gpx1000 @SaschaWillems I want to go ahead and merge this and #207 because they both fix basic wrong things and I don't want to wait until next week's docs call to do another site update. If you object to that please speak up soon. The behavior @SaschaWillems describes above is AFAICT an artifact of how the preview site is trying to remove duplicates and expected - the first click on the PR-206 nav index correctly resolves to a refpage, but the refpage it resolves to is from main branch CI, not this branch, so it's expected it would then fail. It does not happen locally and should not on docs.vulkan.org, though I will check that. |
Rewrites captured nav hrefs/src to be component-root-relative instead of page-relative. The shared nav loader now accepts a per-page prefix that adjusts URLs back to each page's URL space, preventing duplication bugs like
/source/source/when nested pages reference the same shared nav.