Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

Conversation

topherfangio
Copy link
Contributor

Currently, the navigation groups on the left would have their icon appear to be toggled open, but the actual content would be invisible because the element's height was not being properly set.

Fix by setting the height in a $mdUtil.nextTick() so that it fires at the appropriate time (and can measure the height).

Additionally, add some code to scroll the selected item into view so that users know which page they are visiting.

Fixes #8841.

@topherfangio topherfangio added the needs: review This PR is waiting on review from the team label Jun 23, 2016
@topherfangio
Copy link
Contributor Author

@crisbeto @EladBezalel @devversion Can you guys run this locally and tell me if you have any issues? It was bugging the crap out of me while I'm testing some other stuff, so I took a bit to fix it :-)

@topherfangio topherfangio added type: docs P3: important Important issues that really should be fixed when possible. labels Jun 23, 2016
@devversion
Copy link
Member

@topherfangio Just run it locally and it seems to work pretty nicely.

Also the animated scroll to the current menu-item is working great.

@crisbeto
Copy link
Member

I'll give it a try later, but that seems like a lot of timeouts. Did you try with rAF instead?

@topherfangio
Copy link
Contributor Author

@crisbeto I have not tried with rAF yet. I could give that a go and see if I can make it smoother.

@crisbeto
Copy link
Member

Worked pretty good on all browsers @topherfangio. An improvement could be to have some nicer easing when scrolling down. You can check the function that we use on progressCircular. It also has a link where you can generate your own easing functions.

@topherfangio
Copy link
Contributor Author

@crisbeto Thanks for testing it! I'll checkout that easing function and see if I can make things a bit smoother.

@topherfangio topherfangio force-pushed the fix-8841-docs-navigation-toggle branch 2 times, most recently from 5e35102 to 82cd68f Compare June 27, 2016 21:39
@topherfangio
Copy link
Contributor Author

@crisbeto Updated to use the easing function. Think you could give it another quick test?

As a note: I rebased/force-pushed, so you may need to delete/re-checkout the branch.


function calculateNewPosition() {
var duration = 1000;
var currentTime = Date.now() - startTime;
Copy link
Member

Choose a reason for hiding this comment

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

There's $mdUtil.now for getting the timestamp.

Currently, the navigation groups on the left would have their icon
appear to be toggled open, but the actual content would be invisible
because the element's height was not being properly set.

Fix by setting the height in a `$mdUtil.nextTick()` so that it fires
at the appropriate time (and can measure the height).

Additionally, add some code to scroll the selected item into view so
that users know which page they are visiting.

Fixes angular#8841.
@topherfangio
Copy link
Contributor Author

@ThomasBurleson Fixed the few issues @crisbeto mentioned. Ready for your review.

@topherfangio topherfangio added pr: merge ready This PR is ready for a caretaker to review and removed needs: review This PR is waiting on review from the team labels Jun 28, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P3: important Important issues that really should be fixed when possible. pr: merge ready This PR is ready for a caretaker to review type: docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants