Skip to content

Commit

Permalink
🐛 [Amp story] [Draggable drawer] Null check on headerEl. (ampproject#…
Browse files Browse the repository at this point in the history 
…35119)

* Null check on headerEl.

* Remove code unrelated to bug.

* Remove trailing comma from protected variables. Correctly type private variable.
  • Loading branch information
@processprocess
processprocess authored Jul 21, 2021
1 parent 74e6b0c commit bbec8df
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 64 deletions.
77 changes: 37 additions & 40 deletions extensions/amp-story/1.0/amp-story-draggable-drawer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,25 +89,25 @@ export class DraggableDrawer extends AMP.BaseElement {
this.ampComponents_ = [];

/** @protected {?Element} */
this.containerEl_ = null;
this.containerEl = null;

/** @protected {?Element} */
this.contentEl_ = null;
this.contentEl = null;

/** @private {number} Max value in pixels that can be dragged when opening the drawer. */
this.dragCap_ = Infinity;

/** @protected {?Element} */
this.headerEl_ = null;
this.headerEl = null;

/** @private {boolean} */
this.ignoreCurrentSwipeYGesture_ = false;

/** @protected {!DrawerState} */
this.state_ = DrawerState.CLOSED;
this.state = DrawerState.CLOSED;

/** @protected @const {!./amp-story-store-service.AmpStoryStoreService} */
this.storeService_ = getStoreService(this.win);
this.storeService = getStoreService(this.win);

/** @private {!Object} */
this.touchEventState_ = {
Expand All @@ -127,7 +127,7 @@ export class DraggableDrawer extends AMP.BaseElement {
/**
* For amp-story-page-attachment-ui-v2 experiment
* Used for offsetting drag.
* @protected {?number}
* @private {?number}
*/
this.spacerElHeight_ = null;
}
Expand All @@ -143,15 +143,15 @@ export class DraggableDrawer extends AMP.BaseElement {

const templateEl = getTemplateEl(this.element);
const headerShadowRootEl = this.win.document.createElement('div');
this.headerEl_ = getHeaderEl(this.element);
this.headerEl = getHeaderEl(this.element);

createShadowRootWithStyle(headerShadowRootEl, this.headerEl_, CSS);
createShadowRootWithStyle(headerShadowRootEl, this.headerEl, CSS);

this.containerEl_ = dev().assertElement(
this.containerEl = dev().assertElement(
templateEl.querySelector('.i-amphtml-story-draggable-drawer-container')
);
this.contentEl_ = dev().assertElement(
this.containerEl_.querySelector(
this.contentEl = dev().assertElement(
this.containerEl.querySelector(
'.i-amphtml-story-draggable-drawer-content'
)
);
Expand All @@ -170,10 +170,10 @@ export class DraggableDrawer extends AMP.BaseElement {
);
spacerEl.setAttribute('aria-label', localizedCloseString);
}
this.containerEl_.insertBefore(spacerEl, this.contentEl_);
this.contentEl_.appendChild(headerShadowRootEl);
this.containerEl.insertBefore(spacerEl, this.contentEl);
this.contentEl.appendChild(headerShadowRootEl);
this.element.classList.add('i-amphtml-amp-story-page-attachment-ui-v2');
this.headerEl_.classList.add('i-amphtml-amp-story-page-attachment-ui-v2');
this.headerEl.classList.add('i-amphtml-amp-story-page-attachment-ui-v2');
} else {
templateEl.insertBefore(headerShadowRootEl, templateEl.firstChild);
}
Expand Down Expand Up @@ -206,7 +206,7 @@ export class DraggableDrawer extends AMP.BaseElement {
* @protected
*/
initializeListeners_() {
this.storeService_.subscribe(
this.storeService.subscribe(
StateProperty.UI_STATE,
(uiState) => {
this.onUIStateUpdate_(uiState);
Expand All @@ -226,7 +226,7 @@ export class DraggableDrawer extends AMP.BaseElement {

// For displaying sticky header on mobile.
new this.win.IntersectionObserver((e) => {
this.headerEl_.classList.toggle(
this.headerEl.classList.toggle(
'i-amphtml-story-draggable-drawer-header-stuck',
!e[0].isIntersecting
);
Expand All @@ -241,9 +241,9 @@ export class DraggableDrawer extends AMP.BaseElement {
this.element.addEventListener('transitionend', (e) => {
if (
e.propertyName === 'transform' &&
this.state_ === DrawerState.CLOSED
this.state === DrawerState.CLOSED
) {
this.containerEl_./*OK*/ scrollTop = 0;
this.containerEl./*OK*/ scrollTop = 0;
}
});
}
Expand All @@ -261,7 +261,7 @@ export class DraggableDrawer extends AMP.BaseElement {
? this.startListeningForTouchEvents_()
: this.stopListeningForTouchEvents_();

this.headerEl_.toggleAttribute('desktop', !isMobile);
this.headerEl.toggleAttribute('desktop', !isMobile);
}

/**
Expand Down Expand Up @@ -353,10 +353,7 @@ export class DraggableDrawer extends AMP.BaseElement {
this.touchEventState_.swipingUp = y < this.touchEventState_.lastY;
this.touchEventState_.lastY = y;

if (
this.state_ === DrawerState.CLOSED &&
!this.touchEventState_.swipingUp
) {
if (this.state === DrawerState.CLOSED && !this.touchEventState_.swipingUp) {
return;
}

Expand Down Expand Up @@ -390,8 +387,8 @@ export class DraggableDrawer extends AMP.BaseElement {
*/
shouldStopPropagation_() {
return (
this.state_ !== DrawerState.CLOSED ||
(this.state_ === DrawerState.CLOSED && this.touchEventState_.swipingUp)
this.state !== DrawerState.CLOSED ||
(this.state === DrawerState.CLOSED && this.touchEventState_.swipingUp)
);
}

Expand Down Expand Up @@ -436,7 +433,7 @@ export class DraggableDrawer extends AMP.BaseElement {

// If the drawer is open, figure out if the user is trying to scroll the
// content, or actually close the drawer.
if (this.state_ === DrawerState.OPEN) {
if (this.state === DrawerState.OPEN) {
const isContentSwipe = this.isDrawerContentDescendant_(
dev().assertElement(gesture.event.target)
);
Expand All @@ -449,7 +446,7 @@ export class DraggableDrawer extends AMP.BaseElement {
// dragging/closing the drawer.
if (
(isContentSwipe && deltaY < 0) ||
(isContentSwipe && deltaY > 0 && this.containerEl_./*OK*/ scrollTop > 0)
(isContentSwipe && deltaY > 0 && this.containerEl./*OK*/ scrollTop > 0)
) {
this.ignoreCurrentSwipeYGesture_ = true;
return;
Expand All @@ -459,13 +456,13 @@ export class DraggableDrawer extends AMP.BaseElement {
gesture.event.preventDefault();

if (data.last === true) {
if (this.state_ === DrawerState.DRAGGING_TO_CLOSE) {
if (this.state === DrawerState.DRAGGING_TO_CLOSE) {
!swipingUp && deltaY > TOGGLE_THRESHOLD_PX
? this.close_()
: this.open();
}

if (this.state_ === DrawerState.DRAGGING_TO_OPEN) {
if (this.state === DrawerState.DRAGGING_TO_OPEN) {
swipingUp && -deltaY > TOGGLE_THRESHOLD_PX
? this.open()
: this.close_();
Expand All @@ -475,7 +472,7 @@ export class DraggableDrawer extends AMP.BaseElement {
}

if (
this.state_ === DrawerState.DRAGGING_TO_OPEN &&
this.state === DrawerState.DRAGGING_TO_OPEN &&
swipingUp &&
-deltaY > this.openThreshold_
) {
Expand Down Expand Up @@ -530,13 +527,13 @@ export class DraggableDrawer extends AMP.BaseElement {
drag_(deltaY) {
let translate;

switch (this.state_) {
switch (this.state) {
case DrawerState.CLOSED:
case DrawerState.DRAGGING_TO_OPEN:
if (deltaY > 0) {
return;
}
this.state_ = DrawerState.DRAGGING_TO_OPEN;
this.state = DrawerState.DRAGGING_TO_OPEN;
let drag = Math.max(deltaY, -this.dragCap_);
if (isPageAttachmentUiV2ExperimentOn(this.win)) {
drag -= this.spacerElHeight_;
Expand All @@ -548,7 +545,7 @@ export class DraggableDrawer extends AMP.BaseElement {
if (deltaY < 0) {
return;
}
this.state_ = DrawerState.DRAGGING_TO_CLOSE;
this.state = DrawerState.DRAGGING_TO_CLOSE;
translate = `translate3d(0, ${deltaY}px, 0)`;
break;
}
Expand All @@ -567,13 +564,13 @@ export class DraggableDrawer extends AMP.BaseElement {
* @param {boolean=} shouldAnimate
*/
open(shouldAnimate = true) {
if (this.state_ === DrawerState.OPEN) {
if (this.state === DrawerState.OPEN) {
return;
}

this.state_ = DrawerState.OPEN;
this.state = DrawerState.OPEN;

this.storeService_.dispatch(Action.TOGGLE_PAUSED, true);
this.storeService.dispatch(Action.TOGGLE_PAUSED, true);

this.mutateElement(() => {
this.element.setAttribute('aria-hidden', false);
Expand All @@ -587,7 +584,7 @@ export class DraggableDrawer extends AMP.BaseElement {
}

this.element.classList.add('i-amphtml-story-draggable-drawer-open');
toggle(dev().assertElement(this.containerEl_), true);
toggle(dev().assertElement(this.containerEl), true);
}).then(() => {
const owners = Services.ownersForDoc(this.element);
owners.scheduleLayout(this.element, this.ampComponents_);
Expand All @@ -610,13 +607,13 @@ export class DraggableDrawer extends AMP.BaseElement {
* @protected
*/
closeInternal_(shouldAnimate = true) {
if (this.state_ === DrawerState.CLOSED) {
if (this.state === DrawerState.CLOSED) {
return;
}

this.state_ = DrawerState.CLOSED;
this.state = DrawerState.CLOSED;

this.storeService_.dispatch(Action.TOGGLE_PAUSED, false);
this.storeService.dispatch(Action.TOGGLE_PAUSED, false);

this.mutateElement(() => {
this.element.setAttribute('aria-hidden', true);
Expand Down
Loading

0 comments on commit bbec8df

Please sign in to comment.