-
-
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
docs: Improve Quick start instructions #1458
Conversation
- Slightly tweak Python server instructions to be more generic - Clarify the section about customizing the main container
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/docsify-core/docsify-preview/esq553l1l |
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 64de258:
|
@@ -97,7 +97,7 @@ If you want, you can show a loading dialog before docsify starts to render your | |||
<div id="app">Please wait...</div> | |||
``` | |||
|
|||
You should set the `data-app` attribute if you changed `el`: | |||
By default, the `id` attribute of the main container is `app`. If you want to use a different id, you must also set the `data-app` attribute, and [configure](configuration.md#el) the `el` parameter 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.
A bit weird that we need to set data-app
like this. Let's fix in v5.
/wish-there-was-a-bot-command-to-open-an-new-issue-right-from-here
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.
I added it manually https://github.com/docsifyjs/docsify/projects/2#card-51972364
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.
@sy-records let's get things moving. Let's make changes more organic (as per the other v5 topic I closed), and we can release a major version bump any time we fix a small feature that is breaking. In this case, this one is worth it in my opinion.
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.
Thanks for this!
This reverts commit 9a15ffb.
Sorry, The branch is wrong, We didn't notice... @trusktr 😅 |
I see that I noticed the
I'd be happy to submit a PR fixing those, if that list is exhaustive, and hardcoding |
Oh, I see, thanks for the feedback @waldyrious |
@waldyrious That would be great indeed to update those. Not sure about the dynamic part. Maybe GitHub had an API to fetch the default branch? EDIT: looks like it does! See here: https://docs.github.com/en/free-pro-team@latest/rest/reference/repos#get-a-repository, in particular the Here's the example payload for Docsify: |
@waldyrious Can you resubmit a PR for #1458? |
@trusktr thanks for the information! I took a look at the code but it's a little more complex than I anticipated at first. I suppose one would have to use the get function from
@sy-records sure! I just submitted #1466. |
Summary
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.