Navigation style and experience improvements #870

Merged
merged 4 commits into from Aug 10, 2015

Conversation

Projects
None yet
2 participants
@kabel
Contributor

kabel commented Jul 23, 2015

Fixes the following navigation issues:

  • Content jumps in mobile when the navigation bar floats
  • Swipe to open nav has not worked out (opens at the wrong time)
  • Poor implementations of throttle and debounce for DOM events.

kabel added some commits Jul 23, 2015

Fix mobile nav float causes content jump
In the mobile presentation, when the navigation toggle is floated, the
rest of the content jumps up to fill in the space abandoned by the nav.
Remove swipe listeners from navigation
This experiement in user experience caused too many problems with the
navigation opening at all the wrong times.
Use real throttle and debounce method in navigation events
Use the throttle and debounce implementations from Underscore to handle
the scroll and resize DOM events in a more standard manner.
@kabel

This comment has been minimized.

Show comment
Hide comment
@kabel

kabel Aug 10, 2015

Contributor

👍 or 👎

Contributor

kabel commented Aug 10, 2015

👍 or 👎

+ var timeout = null;
+ var previous = 0;
+ if (!options) options = {};
+ var later = function() {

This comment has been minimized.

@mfairchild365

mfairchild365 Aug 10, 2015

Member

This looks good, but I have two thoughts. These functions are pretty hard to follow, especially if you are new to them and what they are supposed to do. I think some comments could really add to the readability. For example, what is the purpose of previous and what sort of values should it contain? what is later used for?

I could also see unit tests be very helpful for functions like these.

@mfairchild365

mfairchild365 Aug 10, 2015

Member

This looks good, but I have two thoughts. These functions are pretty hard to follow, especially if you are new to them and what they are supposed to do. I think some comments could really add to the readability. For example, what is the purpose of previous and what sort of values should it contain? what is later used for?

I could also see unit tests be very helpful for functions like these.

This comment has been minimized.

@kabel

kabel Aug 10, 2015

Contributor

Good point. These are pulled directly from vendor code. I will add a comment to that extent.

@kabel

kabel Aug 10, 2015

Contributor

Good point. These are pulled directly from vendor code. I will add a comment to that extent.

@mfairchild365

This comment has been minimized.

Show comment
Hide comment
@mfairchild365

mfairchild365 Aug 10, 2015

Member

👍

I ran it locally and it appeared to work beautifully.

Member

mfairchild365 commented Aug 10, 2015

👍

I ran it locally and it appeared to work beautifully.

Add attribution for lib functions from Underscore.js
The throttle/debounce functions from Underscore.js have been included.

kabel added a commit that referenced this pull request Aug 10, 2015

Merge pull request #870 from kabel/bugfix-nav-scroll
Navigation style and experience improvements

@kabel kabel merged commit 9c88d64 into unl:develop Aug 10, 2015

@kabel kabel deleted the kabel:bugfix-nav-scroll branch Aug 10, 2015

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