search

dhis2 / ui-core

:no_entry: [DEPRECATED] Please refer to https://github.com/dhis2/ui
BSD 3-Clause "New" or "Revised" License
8 stars 1 forks source link

docs: adds prefix to jsdocs link since they were returning 404 #877

Closed paschalidi closed 4 years ago

paschalidi commented 4 years ago

👋 hello!

This is my first PR without opening an issue. Maybe I should have opened one first. Feel free to give me your feedback both on the code and the procedure anyways!

The issue description:

While I was navigating at the ui-core docs I realised that clicking on the Live demo link would return a 404 page instead of the story. image

The problem is that we are having this /#/ on the url and therefore the link is broken. I added the full link to fix this issue however I can imagine that there might be a better solution to this! Le me know your thoughts and feedback!

cypress[bot] commented 4 years ago


Test summary

258 • 0 • 0 • 0

Run details
Project ui-core
Status Passed
Commit 40410a4146
Started Apr 14, 2020 3:56 PM
Ended Apr 14, 2020 4:02 PM
Duration 06:39 💡
OS Linux Ubuntu Linux - 18.04
Browser Electron 78

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

ismay commented 4 years ago

Thanks! I guess the hash is considered part of the root, and then it appends the root relative url to that? The fix does mean that the links in the api docs will now always point to the live production docs. So the links in PR builds will now also point to the 'production' docs, whereas previously they'd link to the docs from the PR. So:

I haven't looked into what's going on with the jsdocs. Maybe the routing there could be changed from hash routing? That'd allow us to get rid of the hash.

ismay commented 4 years ago

The documentation build is done by https://github.com/dhis2/cli-utils-docsite, so if we want to change anything I assume we'd have to change it there. I'm sure of the history of our doc builds though, and what the plans are. I think @amcgee and @varl know more about that.

paschalidi commented 4 years ago

Thanks! I guess the hash is considered part of the root, and then it appends the root relative url to that? The fix does mean that the links in the api docs will now always point to the live production docs. So the links in PR builds will now also point to the 'production' docs, whereas previously they'd link to the docs from the PR. So:

I haven't looked into what's going on with the jsdocs. Maybe the routing there could be changed from hash routing? That'd allow us to get rid of the hash.

Yeah you right havent thought of that. Apologies.

Best would be as you say to remove the hash. However, do know @ismay what is the use of the hash?

ismay commented 4 years ago

Actually, my assumption was wrong. The problem is with the generated html. It's generated by https://docsify.js.org, which prepends the hash, so it turns this: 

/demo/?path=/story/alertbar--default|Storybook

into this:

<a href="#/demo/?path=%2fstory%2falertbar--default">Storybook</a>

The hash is used for docsify's routing, but it shouldn't be prepended for these links. I'll look into if it can be omitted somehow. Will report back.

paschalidi commented 4 years ago

Thanks for taking the initiative! Will be willing to help dont hesitate to ask

ismay commented 4 years ago

I think that this is what we're looking for https://docsify.js.org/#/configuration?id=nocompilelinks. And our docsify configuration can be found here: https://github.com/dhis2/cli-utils-docsite/blob/master/template/js/initDocsify.js

I assume that setting:

noCompileLinks: ['/demo/.*']

would solve our issue. (Or something of that nature, I don't know precisely what glob syntax they expect)

We could also change the router mode, but that would require us to configure redirects, so the above might be the simplest option.

ismay commented 4 years ago

@paschalidi I'm hoping that this'll do the trick: https://github.com/dhis2/cli-utils-docsite/pull/47

paschalidi commented 4 years ago

cool! I am closing this! Thanks for taking care of it!!

ismay commented 4 years ago

cool! I am closing this! Thanks for taking care of it!!

N.p., thanks for picking this up (I'm not sure whether my proposed fix is how we want to fix it btw., but we'll see).

Thanks again for the PR, it's easy to miss these kinds of hiccups with documentation, much appreciated 👍!