Skip to content

Conversation

prateekbh
Copy link
Contributor

This adds the capability for the navigation menu to open and close by swiping in either direction.

This also takes care that a diagonal swipe should result in only one of the two things
wither horizontal navigation close/open or vertical scroll.

@jsf-clabot
Copy link

jsf-clabot commented Nov 19, 2016

CLA assistant check
All committers have signed the CLA.

@skipjack
Copy link
Collaborator

Tested this out, looks good thank you! I would just ask that you continue to follow BEM for the new elements you added and sign the CLA. Once that's done this should be good to go.

@prateekbh
Copy link
Contributor Author

@skipjack the opener element right?
I guess sidebar-content should do good.

@skipjack
Copy link
Collaborator

@prateekbh it would be both. Basically, with BEM and react, each component is a "block" (in this case sidebar-mobile) and every other element is classed within that block so it has to be sidebar-mobile__***. Especially in this case where we have another Sidebar component thus making the sidebar-content somewhat confusing.

@skipjack
Copy link
Collaborator

Hey @prateekbh, hope you don't mind I pushed the fixes to your fork. Would you mind signing the CLA? Once that's done that I can merge 👍

@prateekbh
Copy link
Contributor Author

Hey @skipjack, done :)
Thanks for fixing the stuff

@prateekbh
Copy link
Contributor Author

Also please let me know when will it be deployed?

@skipjack
Copy link
Collaborator

Closing and re-opening with some last fixes and resolving of merge conflicts. @prateekbh thank you, I'll be merging the updated PR momentarily.

I don't do the deployments, but I suspect you'll see a PR called "Deploy" or "Publish" pop up this week -- once that's merged you should see the live site updated with these changes and others.

@skipjack skipjack closed this Nov 21, 2016
skipjack added a commit that referenced this pull request Nov 21, 2016
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.

3 participants