Skip to content
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

refactor: refactoring hideSidebar configuration. #1396

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,10 @@ window.$docsify = {
## hideSidebar

- Type : `Boolean`
- Default: `true`
- Default: `false`

This option will completely hide your sidebar and won't render any content on the side.
This option will completely hide your sidebar and won't render any content on the side.
Meanwhile, the search plugin won't work either.

```js
window.$docsify = {
Expand Down
1 change: 1 addition & 0 deletions src/core/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export default function(vm) {
subMaxLevel: 0,
loadSidebar: null,
loadNavbar: null,
hideSidebar: false,
homepage: 'README.md',
coverpage: '',
basePath: '',
Expand Down
27 changes: 20 additions & 7 deletions src/core/fetch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,11 @@ export function fetchMixin(proto) {
return path404;
};

proto._loadSideAndNav = function(path, qs, loadSidebar, cb) {
proto._loadSideAndNav = function(path, qs, loadSidebar, hideSidebar, cb) {
return () => {
if (hideSidebar) {
return;
}
if (!loadSidebar) {
return cb();
}
Expand All @@ -104,7 +107,12 @@ export function fetchMixin(proto) {
proto._fetch = function(cb = noop) {
const { path, query } = this.route;
const qs = stringifyQuery(query, ['id']);
const { loadNavbar, requestHeaders, loadSidebar } = this.config;
const {
loadNavbar,
requestHeaders,
loadSidebar,
hideSidebar,
} = this.config;
// Abort last request

const file = this.router.getFile(path);
Expand All @@ -120,7 +128,7 @@ export function fetchMixin(proto) {
this._renderMain(
text,
opt,
this._loadSideAndNav(path, qs, loadSidebar, cb)
this._loadSideAndNav(path, qs, loadSidebar, hideSidebar, cb)
),
_ => {
this._fetchFallbackPage(path, qs, cb) || this._fetch404(file, qs, cb);
Expand Down Expand Up @@ -197,7 +205,12 @@ export function fetchMixin(proto) {
};

proto._fetchFallbackPage = function(path, qs, cb = noop) {
const { requestHeaders, fallbackLanguages, loadSidebar } = this.config;
const {
requestHeaders,
fallbackLanguages,
loadSidebar,
hideSidebar,
} = this.config;

if (!fallbackLanguages) {
return false;
Expand All @@ -219,7 +232,7 @@ export function fetchMixin(proto) {
this._renderMain(
text,
opt,
this._loadSideAndNav(path, qs, loadSidebar, cb)
this._loadSideAndNav(path, qs, loadSidebar, hideSidebar, cb)
),
() => this._fetch404(path, qs, cb)
);
Expand Down Expand Up @@ -255,7 +268,7 @@ export function fetchMixin(proto) {
}

export function initFetch(vm) {
const { loadSidebar } = vm.config;
const { loadSidebar, hideSidebar } = vm.config;

// Server-Side Rendering
if (vm.rendered) {
Expand All @@ -264,7 +277,7 @@ export function initFetch(vm) {
activeEl.parentNode.innerHTML += window.__SUB_SIDEBAR__;
}

vm._bindEventOnRendered(activeEl);
!hideSidebar && vm._bindEventOnRendered(activeEl);
vm.$resetEvents();
callHook(vm, 'doneEach');
callHook(vm, 'ready');
Expand Down
15 changes: 2 additions & 13 deletions src/core/render/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function renderMain(html) {
this._renderTo('.markdown-section', html);

// Render sidebar with the TOC
!this.config.loadSidebar && this._renderSidebar();
!this.config.hideSidebar && !this.config.loadSidebar && this._renderSidebar();

// Execute markdown <script>
if (
Expand Down Expand Up @@ -128,18 +128,7 @@ export function renderMixin(proto) {
};

proto._renderSidebar = function(text) {
const { maxLevel, subMaxLevel, loadSidebar, hideSidebar } = this.config;

if (hideSidebar) {
// FIXME : better styling solution
document.querySelector('aside.sidebar').remove();
document.querySelector('button.sidebar-toggle').remove();
document.querySelector('section.content').style.right = 'unset';
document.querySelector('section.content').style.left = 'unset';
document.querySelector('section.content').style.position = 'relative';
document.querySelector('section.content').style.width = '100%';
Comment on lines -137 to -140
Copy link
Member

Choose a reason for hiding this comment

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

@anikethsaha Should we keep the style?

develop branch

image

now

image

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we can just keep this configurations without adjusting much styles right now (dont let render code including much styles), and if there has much users proposal to make it that we can refine it again.
WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep them and not change them in the z versions.

return null;
}
const { maxLevel, subMaxLevel, loadSidebar } = this.config;

this._renderTo('.sidebar-nav', this.compiler.sidebar(text, maxLevel));
const activeEl = getAndActive(this.router, '.sidebar-nav', true, true);
Expand Down
3 changes: 2 additions & 1 deletion src/core/render/tpl.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ export function main(config) {
'<div class="sidebar-nav"><!--sidebar--></div>' +
'</aside>';
return (
`<main>${aside}` +
'<main>' +
(!config.hideSidebar ? `${aside}` : '') +
'<section class="content">' +
'<article class="markdown-section" id="main"><!--main--></article>' +
'</section>' +
Expand Down
16 changes: 0 additions & 16 deletions test/e2e/sidebar.test.js

This file was deleted.

69 changes: 69 additions & 0 deletions test/integration/configuration.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
const docsifyInit = require('../helpers/docsify-init');

// Suite
// -----------------------------------------------------------------------------
describe('Configuration Tests', function() {
// Tests
// ---------------------------------------------------------------------------
test('Configuration hideSidebar', async () => {
// Init docsify basic fully site
const docsifyInitConfig = {
config: {
name: 'Docsify Name',
hideSidebar: true,
},
markdown: {
coverpage: `
# Docsify Test

> Testing a magical documentation site generator

[GitHub](https://github.com/docsifyjs/docsify/)
`,
homepage: `
# Hello World

This is the homepage.
`,
navbar: `
- [docsify.js.org](https://docsify.js.org/#/)
`,
sidebar: `
- [Test Page](test)
`,
},
routes: {
'/test.md': `
# Test Page

This is a custom route.
`,
'/data-test-scripturls.js': `
document.body.setAttribute('data-test-scripturls', 'pass');
`,
},
script: `
document.body.setAttribute('data-test-script', 'pass');
`,
scriptURLs: ['/data-test-scripturls.js', '/lib/plugins/search.min.js'],
style: `
body {
background: red !important;
}
`,
styleURLs: ['/lib/themes/vue.css'],
};

await docsifyInit({
...docsifyInitConfig,
});

// Verify config options
expect(typeof window.$docsify).toEqual('object');
expect(window.$docsify).toHaveProperty('hideSidebar', true);

// Verify sidebar not exist
expect(document.querySelector('aside.sidebar')).toBeNull();
expect(document.querySelector('button.sidebar-toggle')).toBeNull();
});
});