-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
fix: Updated docs with instructions for installing specific version #780 #1225
fix: Updated docs with instructions for installing specific version #780 #1225
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/docsify-core/docsify-preview/g5aps4t4l |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 352aac6:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two change requests:
-
Remove the file path and name from the URL:
<script src="//cdn.jsdelivr.net/npm/docsify@4"></script>
JSDelivr will automatically deliver the file specified in
package.json
using thejsdelivr
,browser
ormain
property and minify it if necessary. Docsify specifies"main": "lib/docsify.js"
in its package.json file, so specifying only//cdn.jsdelivr.net/npm/docsify@4
will result in the minified version oflib/docsify.js
being delivered. The reason to do this is because it gives maintainers the ability to change the file location of the main file without requiring an update to the CDN URL. If interested, you can read more about it here: https://www.jsdelivr.com/features#publishing-packages -
Add a docsify version to the stylesheet as well
Should be:
<link rel="stylesheet" href="//cdn.jsdelivr.net/npm/docsify@4/lib/themes/vue.css">
Note here that, unlike above, the full URL (path and filename) are required. This is because only one default file can be specified in
package.json
.
@jhildenbiddle Just pushed the requested changes. Also updated docsify-cli#107 accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good, but after reading the full doc I realized we need additional changes.
The "Manual Installation" section now contains two blocks of HTML: the first with and the second without @4
in docsify-related URLs. We only want one block of HTML, and that should be the one with @4
in the URLs since the whole point of this change is to lock users to v4 so their site doesn't break when the next major version is released.
Content should be as follow:
Manual initialization
If you don't like
npm
or have trouble installing the tool, you can manually createindex.html
:[Updated HTML with docsify version in URLs]
If you installed python on your system, you can easily use it to run a static server to preview your site.
cd docs && python -m SimpleHTTPServer 3000
@aemmadi thanks for making this PR! Besides having only one HTML block I think we should still explicitly mention in the wording outside of the block to highly recommend having a version number. @aemmadi mind making one more change? to add a note about using the version so that updates "don't break your website"? |
Good point, @trusktr. A description of why docsify URLs are locked to a specific version ( Specifying docsify versionsSpecifying a major version in the URL ( <link rel="stylesheet" href="//cdn.jsdelivr.net/npm/docsify@4/themes/vue.css">
<script src="//cdn.jsdelivr.net/npm/docsify@4"></script> If you prefer to lock docsify to a specific version, specify the full version after the <link rel="stylesheet" href="//cdn.jsdelivr.net/npm/docsify@4.11.4/themes/vue.css">
<script src="//cdn.jsdelivr.net/npm/docsify@4.11.4"></script> ?> Note that in both of the examples above, docsify URLs will need to be manually updated when a new major version of docsify is released (e.g. v4.x.x => v5.x.x). Check the docsify website periodically to see if a new major version has been released. Thoughts? Too wordy? BTW, I'm happy to update the docs in a separate PR is that is preferred. |
I noticed you used the full file path in So the <script src="//cdn.jsdelivr.net/npm/docsify@4"></script>
<script src="//cdn.jsdelivr.net/npm/docsify@4.11.4"></script> |
@aemmadi -- Good catch. Sorry about that. Copy/Paste error. I've updated my comment by removing the full path to |
I added all the requested changes and made sub headings to make the doc look aesthetic. Let me know if that works! |
cc @jhildenbiddle can you review this as there is one change request from you. @aemmadi can you rebase with the upstream . Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @aemmadi --
Looks like the wrong block of HTML was deleted. We want to keep the one with the @4
in the URLs.
@jhildenbiddle Pushed the changes! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Nice work, @aemmadi!
idk whats wrong but I cant able to update the branch, @aemmadi can you update your branch . |
Weird, I updated it to the |
* develop: docs: removed codefund docs and plugin (#1262) docs: remove bundle size from the home page and documentation (#1257) fix: search can not search the table header (#1256) fix: after setting the background image, the button is obscured (#1234) Fix: fixed onlycover flag in mobile (#1243) fix: Updated docs with instructions for installing specific version (fixes #780) (#1225) fix: Add error handling for missing dependencies (fixes #1210) (#1232) [documdocs: deploy docsify in docker. (#1241) docs: Add embed gist instructions to Embed Files (fixes #932 ) (#1238) chore: add changelog 4.11.4 [build] 4.11.4 feat: added html sanitizer for remote rendering (#1128)
Summary
Updated docs with example of using a specific version of docsify while getting started, instead of the default version.
Issue #780 requested changes in the CLI, opened a PR there as well with the changes #107
What kind of change does this PR introduce? (check at least one)
If changing the UI of default theme, please provide the before/after screenshot:
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
fix #xxx[,#xxx]
, where "xxx" is the issue number)You have tested in the following browsers: (Providing a detailed version will be better.)
If adding a new feature, the PR's description includes:
To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.
Other information:
lib
directory.