Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[do not merge] add a rightSidebar option#1288

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
trusktr wants to merge2 commits intodevelop
base:develop
Choose a base branch
Loading
fromright-sidebar

Conversation

@trusktr
Copy link
Member

Summary

Allows the sidebar to be on the right side. This is something developers might want (f.e. to match UX patterns on other parts of a site).

When sidebar is on the right the navbar and github corner badge are on the left on desktop. No changes to navbar or github corner on mobile (because on mobile they are hidden when the menu is open).

  • This should not be merged until the options I added to index.html files are removed. I added the options so that you can test with vercel. After review, I will remove the options.

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of default theme, please provide thebefore/after screenshot:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g.fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

If adding anew feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated (perhaps not enough tests, but we should settle on testing first, i.e. playwright PR)

To avoid wasting your time, it's best to open afeature request issue first and wait for approval before working on it.

@trusktrtrusktr added enhancement docsrelated to the documentation of docsify itself do not merge semver-minorThis needs a semver-minor release labelsJul 16, 2020
@vercel
Copy link

vercelbot commentedJul 16, 2020•
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect:https://vercel.com/docsify-core/docsify-preview/hkptkpq47
✅ Preview:https://docsify-preview-git-right-sidebar.docsify-core.vercel.app

@trusktrtrusktr changed the titleadd a rightSidebar option[do not merge] add a rightSidebar optionJul 16, 2020

- Type :`Boolean`
- Default:`true`
- Default:`false`
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This was wrong

@codesandbox-ci
Copy link

codesandbox-cibot commentedJul 16, 2020•
edited
Loading

This pull request is automatically built and testable inCodeSandbox.

To see build info of the built libraries, clickhere or the icon next to each commit SHA.

Latest deployment of this branch, based on commit8e98cb8:

SandboxSource
docsify-templateConfiguration

* develop:  remove `lib` from pull request template as it is gitignored  fix: gitignore was ignoring folders in src, so VS Code search results or file fuzzy finder were not working, etc
&>ul
padding-inline-start:0;
padding-inline-end:40px;// Chrome's default value.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@jhildenbiddle I aimed to make all the changes in here non-breaking for the end user.

You may have some opinion on how better to organize this change. Happy to hear if you do!

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The reason forpadding-inline-end: 40px; thing is to flip it to the other side when the menu is on the left, otherwise it has a default style in Chrome's user-agent style ofpadding-inline-start: 40px;.

Maybepadding-inline-start: 40px should be hard-coded for nonright-sidebar` mode, otherwise that one can vary from browser to browser (I don't know if they settled on the same values, haven't checked).

@jhildenbiddle
Copy link
Member

Is the intention to target this for v4, or as a placeholder for v5? If v4, then my questions / concerns outlined in the original issue (#1282) remain:

I think this is a feature best targeted for v5 and discussed in that context.

There's more to consider than just making the menu appear on the right:

  • How are other docsify themes affected?
  • How are third-party themes affected?
  • How are third-party plugins affected (i.e. do any rely on the sidebar-on-left behavior)?
  • If the sidebar slides in from the right, should the sidebar toggle also be located on the right?
  • How does this behavior look at desktop resolutions?

These are just off the top of my head. I'm sure there are more.

More importantly, not every preference needs to be a core docsify feature. Is this feature useful? Possibly, but given the fact that it has never before been requested in the nearly four years that docsify has existed, I think it's safe to say that this is not a high priority feature. I'd look to our existing issues list and feature requests to get a better idea of where we should be focused.

The good news is that folks that want a sidebar on the right can do so today by adding a custom <style> tag to their index.html file with the tweaked values you mentioned above. No need to wait for a new docsify option. We can target this issue for v5, see what kind of activity it gets, and determine if/how to implement this feature in v5 when the time comes.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jhildenbiddlejhildenbiddleAwaiting requested review from jhildenbiddle

@anikethsahaanikethsahaAwaiting requested review from anikethsaha

@Koooooo-7Koooooo-7Awaiting requested review from Koooooo-7

@sy-recordssy-recordsAwaiting requested review from sy-records

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

do not mergedocsrelated to the documentation of docsify itselfenhancementsemver-minorThis needs a semver-minor release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@trusktr@jhildenbiddle
[8]ページ先頭
©2009-2025 Movatter.jp