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

Fix scroll sidebar docs #1492

Merged
merged 6 commits into from Jan 17, 2019

Conversation

@bdlb77
Copy link
Contributor

bdlb77 commented Jan 2, 2019

PR Checklist

What is the current state of the documentation article?

#1455 , implemented scroll effect for both left and right sidebars. Both sidebars didn't have the ability to scroll on content, causing difficulty in UX and navigation on tabs.

What is the new state of the documentation article?

The side navbars should have the ability to scroll to allow for easier access to tabs. Also they are now sticky on the page to always show and have access to their navigations.
Fixes/Implements/Closes #[Issue Number].

Fixes #1455

bdlb77 added 3 commits Jan 2, 2019
Added NsSideScroll function in app.js and appended ns-side-nav-active to the left sidebar.
In the Styles.css included syling for the ns-side-nav-active class  and ns-side-nav

Fix scroll on side navigations
Creation of navigation__right__scroll to right nav container element.
handleScrollNav function created to allow for more visibility while scrolling down on page.
styles applied to navigation__right__scroll class in Styles.css
add #ns-side-scroll for left side scroll

Fix of issue for right side navigation scroll
update pull request with profile to contribution
@etabakov etabakov requested a review from bundyo Jan 3, 2019
@etabakov

This comment has been minimized.

Copy link
Member

etabakov commented Jan 3, 2019

Many thanks for your effort. While reviewing the suggestion I noticed a redundant scrollbar, that ideally shouldn't be there - check the screenshot.

bdlb77 added 2 commits Jan 3, 2019
changed overflow to overflow-y on side-side-nav
Added display none for webkit and firefox to hide scroll bar of right side navigation
@bdlb77

This comment has been minimized.

Copy link
Contributor Author

bdlb77 commented Jan 3, 2019

I found a solution for the navbar that possibly only works for webkit and firefox.. I'm not sure how it looks on IE... maybe y'all have a suggestion on how to implement a fix for it there too :) Thank you guys for this opportunity!
screen shot 2019-01-03 at 2 58 16 pm
screen shot 2019-01-03 at 2 58 23 pm

Copy link
Contributor

bundyo left a comment

I hope the changes I'm requesting are not too much.

Maybe we should have provided a design requirement beforehand. Sorry for missing that.

build/_assets/javascripts/app.js Outdated Show resolved Hide resolved
build/_assets/javascripts/app.js Outdated Show resolved Hide resolved
build/_assets/javascripts/app.js Outdated Show resolved Hide resolved
build/_assets/javascripts/app.js Outdated Show resolved Hide resolved
build/_assets/stylesheets/styles.css Outdated Show resolved Hide resolved
build/_layouts/page.html Outdated Show resolved Hide resolved
build/_assets/stylesheets/styles.css Outdated Show resolved Hide resolved
build/_assets/stylesheets/styles.css Outdated Show resolved Hide resolved
@bdlb77

This comment has been minimized.

Copy link
Contributor Author

bdlb77 commented Jan 9, 2019

So yes you were right.. no JS needed haha.. I made this waaay too complicated than it needed to be. Thank you for the great advice!
I'll add photos for the updated version.

screen shot 2019-01-09 at 2 21 45 pm

screen shot 2019-01-09 at 2 21 37 pm

screen shot 2019-01-09 at 2 21 25 pm

Once I do a full check again I'll push the changes

@bundyo

This comment has been minimized.

Copy link
Contributor

bundyo commented Jan 9, 2019

Thanks.

Btw, looking at your screenshots - the colors of the scrollbar are inverted than what I had in mind - the scroll-thumb should be --border-light-blue and the scroll-track should be transparent.

@bdlb77

This comment has been minimized.

Copy link
Contributor Author

bdlb77 commented Jan 9, 2019

screen shot 2019-01-09 at 3 07 49 pm

Hopefully this one will be what you were lookin for :) sorry for the confusion
Added border-lightbue color to scrollbar thumb and applied styles to right-nav__tree for scroll
Deleted navigation__right__scroll and ns-side-scroll selectors

fixes issue #1455
@bdlb77

This comment has been minimized.

Copy link
Contributor Author

bdlb77 commented Jan 9, 2019

I just pushed the next round of revisions :) gracias

@bundyo
bundyo approved these changes Jan 17, 2019
@bundyo

This comment has been minimized.

Copy link
Contributor

bundyo commented Jan 17, 2019

Thanks for your effort! 👍

@bdlb77

This comment has been minimized.

Copy link
Contributor Author

bdlb77 commented Jan 17, 2019

Thank you guys! I learned lots :)

@etabakov

This comment has been minimized.

Copy link
Member

etabakov commented Jan 17, 2019

Hey, thanks for your contribution! We have a special program for our first-time contributors and you are eligible to participate in it. If you haven't applied already, all you need to do to claim your prize is to fill our Application Form.

Regards from the whole team 🥇

@etabakov etabakov merged commit 00f7deb into NativeScript:master Jan 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.