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

enable sidebar on mobile移动设备上开启侧栏 #1717

Merged
merged 2 commits into from
Jul 10, 2017

Conversation

flashlab
Copy link
Contributor

add a switch onmobile for sidebar in config.
If set, sidebar toggle will display on mobile and tablet view.

@ivan-nginx
Copy link
Collaborator

Wow, good addition. See on your blog from mobile in Mist scheme. Sidebar and b2t is work fine, but there is 1 thing: when you click on sidebar - he's slide frm right. Ok. Now, if i click on emty space (in left) - side bar is slideout. Ok. And if i click on "X" button - sidebar is slideout too. But if i want to make slideout sidebar by slide my finger from left to right, there is no effect. U understand what i meaning? Like on Android, for example. Sideout bu slide your finger from any place in sidebar radius. How i think about this option?

Also, a later will test it on Pisces scheme.

@flashlab
Copy link
Contributor Author

@ivan-nginx It might be a a little complicate to achieve the material-like sidebar, do you have any idea to make it work?

@ivan-nginx
Copy link
Collaborator

@flashlab i think about it, Need some time.

@flashlab
Copy link
Contributor Author

@ivan-nginx Any other problem? hope it'll be merged ASAP.

@ivan-nginx
Copy link
Collaborator

Ok, but need to strongly test it on all schemes to see on bugs.
Also, as i talk above, need to expand this feature.

@ivan-nginx ivan-nginx merged commit 5642d32 into iissnan:master Jul 10, 2017
@flashlab
Copy link
Contributor Author

@ivan-nginx Works fine with Muse and Mist, while pisces has no sidebar. Should be zero-bug if the sidebar keep on the right side.

@flashlab flashlab deleted the patch-1 branch July 11, 2017 03:00
@flashlab
Copy link
Contributor Author

flashlab commented Jul 11, 2017

#1757 implement simple swipable sidebar on touch devices. @ivan-nginx

Copy link
Collaborator

@ivan-nginx ivan-nginx left a comment

Choose a reason for hiding this comment

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

Need to add some switches.

@@ -8,6 +8,7 @@
</div>

<aside id="sidebar" class="sidebar">
<div id="sidebar-dimmer"></div>
Copy link
Collaborator

@ivan-nginx ivan-nginx Jul 11, 2017

Choose a reason for hiding this comment

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

Need to add conditional import there:

{% if theme.onmobile %}
<div id="sidebar-dimmer"></div>
{% endif %}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no need for one line I think.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@import "sidebar-nav";
@import "sidebar-toc";

@import "sidebar-dimmer";
Copy link
Collaborator

@ivan-nginx ivan-nginx Jul 11, 2017

Choose a reason for hiding this comment

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

Need to add conditional import there:
@import "sidebar-dimmer" if hexo-config('onmobile');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not necessary consist with the context

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

@ivan-nginx ivan-nginx left a comment

Choose a reason for hiding this comment

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

And here.

@@ -82,10 +82,12 @@ $(document).ready(function () {

var sidebarToggleMotion = {
toggleEl: $('.sidebar-toggle'),
dimmerEl: $('#sidebar-dimmer'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to add conditional import there:

      if (CONFIG.sidebar.onmobile) {
        ...
      }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no harmful I think

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, right.

sidebarEl: $('.sidebar'),
isSidebarVisible: false,
init: function () {
this.toggleEl.on('click', this.clickHandler.bind(this));
this.dimmerEl.on('click', this.clickHandler.bind(this));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to add conditional import there:

      if (CONFIG.sidebar.onmobile) {
        ...
      }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DITTO

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok.

@ivan-nginx
Copy link
Collaborator

@flashlab and what about Pisces scheme? Looks like sidebar always show at the left side.

@flashlab
Copy link
Contributor Author

@ivan-nginx Pisces has no sidebar, so this'll not work with it.

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Jul 12, 2017

@flashlab Pisces has sidebar too, but this sidebar not same as on Mist and Muse theme.
Ok, i make additions above byself. But in next time need to do conditional import switches, because Next theme is too big with various functions and work slowly. I show u how need to do, u see it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants