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.

Sign up for GitHub

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

Momentum drawer #55

Merged
merged 10 commits into from
Feb 12, 2016
Merged

Momentum drawer #55

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ac8ad6f
Momentum based on track events
keanulee Feb 5, 2016
fd6ebc6
Refactoring and RAF toggling opened state
keanulee Feb 6, 2016
e12bf6c
app-drawer flicking tests
keanulee Feb 9, 2016
80a7cce
Use translate3d()
keanulee Feb 9, 2016
9d648f1
PR fixes
keanulee Feb 10, 2016
57435b8
Replace rAF with custom timed transitions
keanulee Feb 11, 2016
d0749c4
PR fixes
keanulee Feb 11, 2016
67a4ba7
transitionEnd -> transitionend
keanulee Feb 11, 2016
e7ccde2
Update tests
keanulee Feb 12, 2016
93bfa75
Fix potential divide by zero issue
keanulee Feb 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
239 changes: 181 additions & 58 deletions app-drawer/app-drawer.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@

:host([opened]) > #contentContainer {
-webkit-transform: translate3d(0, 0, 0);
transform: translate3d(0, 0, 0);
transform: translate3d(0, 0, 0);
}

#scrim {
Expand Down Expand Up @@ -196,26 +196,26 @@
},

listeners: {
track: '_trackHandler'
track: '_track',
transitionEnd: '_transitionEnd',
webkitTransitionEnd: '_transitionEnd'
Copy link
Contributor

Choose a reason for hiding this comment

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

don't think we need webkitTransitionEnd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed for Safari :(

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to lowercase the event name transitionend and it should work on Chrome, Safari and other supported browsers.

transitionend: '_transitionEnd'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you're right. Thanks.

},

observers: [
'resetLayout(position)',
'_updateDocScroll(opened, persistent)'
'_updateDocScroll(persistent)'
],

_lastTrackDirection: 0,

_translateOffset: 0,

_trackEvents: null,

attached: function() {
// Set the scroll direction so you can vertically scroll inside the drawer.
this.setScrollDirection('y');

/**
* Only transition the drawer shortly after it is attached (e.g. app-drawer-layout
* may need to set the initial opened state which should not be transitioned).
*/
// Only transition the drawer shortly after it is attached (e.g. app-drawer-layout
// may need to set the initial opened state which should not be transitioned).
this.$.contentContainer.style.transition = 'none';
this.async(function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: async doesn't guarantee that you will get a paint before it resets the transition. It could use rFA.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't need to wait for a paint - I only need to wait until app-drawer-layout sets its initial opened state. (I updated the comment style because I thought the /* */ was taking too many lines, and wanted consistent across this file).

Copy link
Contributor

Choose a reason for hiding this comment

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

if it doesn't paint in between those two states, I think it might still animate the drawer on attached.

this.$.contentContainer.style.transition = '';
Expand Down Expand Up @@ -260,75 +260,198 @@

_scrimTapHandler: function() {
if (!this.persistent) {
this.opened = false;
// This debouncer is needed because of Polymer/polymer#3405.
this.debounce('_scrimTapHandler', function() {
this.opened = false;
}, 1);
}
},

_trackHandler: function(event) {
_track: function(event) {
if (!this.persistent) {
// Disable user selection on desktop.
event.preventDefault();
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think calling preventDefault on track event can prevent selection. Usually you will need to call peventDefault on down (or mousedown) event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to work at least when dragging the drawer horizontally on desktop (can still select when dragging vertically). I don't want to disable user selection altogether, but I found that the selection is causing a repaint when I close the drawer on desktop.

I'll update the comment above to specify that this is for desktop.

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good


switch (event.detail.state) {
case 'start':
// Disable transitions since styles will reflect user track events.
this.$.contentContainer.style.transitionDuration = '0s';
this.$.scrim.style.transitionDuration = '0s';
this.$.scrim.style.visibility = 'visible';

if (this.opened) {
this._translateOffset = 0;
} else if (this.position == 'left') {
this._translateOffset = -this.getWidth();
} else if (this.position == 'right') {
this._translateOffset = this.getWidth();
}
this._trackStart(event);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

break;

case 'track':
var translateDelta = event.detail.dx + this._translateOffset;
switch (this.position) {
case 'left':
translateDelta = Math.min(0, translateDelta);
this._translateDrawer(translateDelta);
this.$.scrim.style.opacity = 1 + translateDelta / this.getWidth();
break;
case 'right':
translateDelta = Math.max(0, translateDelta);
this._translateDrawer(translateDelta);
this.$.scrim.style.opacity = 1 - translateDelta / this.getWidth();
break;
}

// Keep track of the last user track event with non-zero ddx (which we
// need to determine whether to keep the drawer open/closed) since the
// very last track event may have zero ddx.
if (event.detail.ddx !== 0) {
this._lastTrackDirection = event.detail.ddx;
}
this._trackMove(event);
break;

case 'end':
this.$.contentContainer.style.transitionDuration = '';
this.$.scrim.style.transitionDuration = '';
this.$.scrim.style.visibility = '';
this.$.scrim.style.opacity = '';
this.transform('', this.$.contentContainer);

// Only toggle the drawer if the user intentionally swipes in one direction.
if (Math.abs(event.detail.dx) > 30) {
this.opened = (this.position === 'left' && this._lastTrackDirection > 0) ||
(this.position === 'right' && this._lastTrackDirection < 0);
}
this._trackEnd(event);
break;
}
}
},

_trackStart: function(event) {
// Disable transitions since style attributes will reflect user track events.
this.$.contentContainer.style.transitionDuration = '0s';
this.$.scrim.style.transitionDuration = '0s';
this.$.scrim.style.visibility = 'visible';

var rect = this.$.contentContainer.getBoundingClientRect();
if (this.position == 'left') {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: this._translateOffset = this.position == 'left' ? rect.left : rect.right - window.innerWidth;

this._translateOffset = rect.left;
} else {
this._translateOffset = rect.right - window.innerWidth;
}

this._trackEvents = [];
},

_trackMove: function(event) {
this._translateDrawer(event.detail.dx + this._translateOffset);
this._trackEvents.push(event);
},

_trackEnd: function(event) {
// Track handler takes precedence over scrim tap handler. See Polymer/polymer#3405.
this.cancelDebouncer('_scrimTapHandler');
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this might not be necessary, same as line 263. The track event should automatically prevent tap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not, at least on Chrome mobile :(

Copy link
Contributor

Choose a reason for hiding this comment

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

@keanulee I recommend talking to @azakus because he told me that these two events are mutually exclusive.

Copy link
Contributor

Choose a reason for hiding this comment

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

@blasten is right. The track event should automatically prevent tap. I did a simple test on Chrome mobile and it seems to do what I expected.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, I could reproduce what @keanulee was seeing. Sounds like a bug in polymer gestures.


// Calculate the velocity.
this._velocity = this._calculateVelocity(event);

// No longer need the track events after calculating velocity - allow them to be GC'd.
this._trackEvents = null;

if (Math.abs(this._velocity) > this.MIN_FLING_VELOCITY) {
// If velocity is above the threshold, fling the drawer in the same direction.
this._flingDrawer(event);
} else {
// Otherwise, toggle the opened state based on the position of the drawer.
var halfWidth = this.getWidth() / 2;
if (event.detail.dx < -halfWidth) {
this.opened = this.position === 'right';
} else if (event.detail.dx > halfWidth) {
this.opened = this.position === 'left';
}
this._resetStyleAttributes();
}
},

_calculateVelocity: function(event) {
// Find the oldest track event that is within 100ms of track end using binary search.
var timeLowerBound = Date.now() - 100;
var trackEvent;
var min = 0;
var max = this._trackEvents.length - 1;
while (min <= max) {
// Floor of average of min and max.
var mid = (min + max) >> 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

Copy link
Contributor

Choose a reason for hiding this comment

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

💯

var e = this._trackEvents[mid];
if (e.timeStamp >= timeLowerBound) {
trackEvent = e;
max = mid - 1;
} else {
min = mid + 1;
}
}
if (trackEvent) {
var dx = event.detail.dx - trackEvent.detail.dx;
var dt = event.timeStamp - trackEvent.timeStamp;
return dx / dt;
}
return 0;
},

_flingDrawer: function(event) {
var x = event.detail.dx + this._translateOffset;
var drawerWidth = this.getWidth();
var isPositionLeft = this.position === 'left';
var isVelocityPositive = this._velocity > 0;
var isClosingLeft = !isVelocityPositive && isPositionLeft;
var isClosingRight = isVelocityPositive && !isPositionLeft;

// Enforce a minimum transition velocity to make the drawer feel snappy.
if (isVelocityPositive) {
this._velocity = Math.max(this._velocity, this.MIN_TRANSITION_VELOCITY);
} else {
this._velocity = Math.min(this._velocity, -this.MIN_TRANSITION_VELOCITY);
}

// Using this timing function, calculate the amount of time needed to finish the
// transition.
const timingFunction = 'cubic-bezier(.667, 1, .667, 1)';
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

const initialSlope = 1.5;
var duration;
if (isClosingLeft) {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing a line between the var declaration and if

duration = (initialSlope * -(x + drawerWidth) / this._velocity) + 'ms';
} else if (isClosingRight) {
duration = (initialSlope * (drawerWidth - x) / this._velocity) + 'ms';
} else {
duration = (initialSlope * -x / this._velocity) + 'ms';
}
this.$.contentContainer.style.transitionDuration = duration;
this.$.contentContainer.style.transitionTimingFunction = timingFunction;
this.$.scrim.style.transitionDuration = duration;
this.$.scrim.style.transitionTimingFunction = timingFunction;

// Transform instead of toggling opened to avoid calling Polymer setters.
if (isClosingLeft) {
this._translateDrawer(-drawerWidth);
} else if (isClosingRight) {
this._translateDrawer(drawerWidth);
} else {
this._translateDrawer(0);
}
},

_transitionEnd: function() {
// Only set the opened state and reset transition timing function if this
// transition end event was the end of a fling transition.
if (this.$.contentContainer.style.transitionTimingFunction) {
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm. another way to do this is check for the event target. wdyt?

_transitionEnd: function(e) {
  if (e.target !== this.$.contentContainer) {
    return;
 }

if (this._velocity < 0) {
this.opened = this.position === 'right';
} else {
this.opened = this.position === 'left';
}

this.$.contentContainer.style.transitionTimingFunction = '';
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we let the bezier easing function as the default?

this.$.scrim.style.transitionTimingFunction = '';
this._resetStyleAttributes();
}

this._updateDocScroll();
},

_resetStyleAttributes: function() {
this.$.contentContainer.style.transitionDuration = '';
this.$.scrim.style.transitionDuration = '';
this.$.scrim.style.visibility = '';
this.$.scrim.style.opacity = '';
this.transform('', this.$.contentContainer);
},

/**
* @return {boolean} True if the given position is valid, false otherwise.
*/
_translateDrawer: function(x) {
this.transform('translate3d(' + x + 'px, 0, 0)', this.$.contentContainer);
var drawerWidth = this.getWidth();
var clampedX = x;
Copy link
Contributor

Choose a reason for hiding this comment

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

how about just var clampedX;


if (this.position === 'left') {
clampedX = Math.max(-drawerWidth, Math.min(x, 0));
this.$.scrim.style.opacity = 1 + clampedX / drawerWidth;
} else {
clampedX = Math.max(0, Math.min(x, drawerWidth));
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to negate clampedX here. e.g. clampedX= -Math.max(0, Math.min(x, drawerWidth)). That way, we will only need one this.$.scrim.style.opacity = 1 + clampedX / drawerWidth; and we could have a ternary operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's either one statement to transform contentContainer or one statement for opacity, but not both. I chose contentContainer.

this.$.scrim.style.opacity = 1 - clampedX / drawerWidth;
}

this.translate3d(clampedX + 'px', '0', '0', this.$.contentContainer);

return (clampedX === x);
Copy link
Contributor

Choose a reason for hiding this comment

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

does _translateDrawer still need to return a boolean?

},

_updateDocScroll: function() {
document.body.style.overflow = (this.persistent || !this.opened) ? '' : 'hidden';
}
},

MIN_FLING_VELOCITY: 0.2,

MIN_TRANSITION_VELOCITY: 1.2

/**
* Fired when the layout of app-drawer has changed.
Expand Down
Loading