Reemerging hint (default to no interaction for 10s)
-
-
-
-
BEFORE
-
AFTER
-
-
-
Reemerging hint (customized to no interaction for 2s)
-
-
-
-
BEFORE
-
AFTER
-
-
-
Instantly reemerging hint (customized to no interaction for 0.001s)
+
No Reemerging hint
-
-
-
BEFORE
-
AFTER
-
-
-
Initial Percent
-
30% to the left
-
-
-
-
-
-
seekTo action
-
-
Position Observer with Seeking (gesture disabled)
-
-
-
-
-
-
-
BEFORE
-
AFTER
-
-
-
-
Example of seeking
-
-
-
-
BEFORE
-
AFTER
-
-
-
-
-
-
-
-
Tab + Keyboard
-
Move step set to 20%, Home for Center, PageUp for Left, PageDown for Right
-
-
-
-
BEFORE
-
AFTER
-
-
-
Inside Carousel
diff --git a/extensions/amp-image-slider/0.1/amp-image-slider.css b/extensions/amp-image-slider/0.1/amp-image-slider.css
index 8de296415e704..a19b88623fca7 100644
--- a/extensions/amp-image-slider/0.1/amp-image-slider.css
+++ b/extensions/amp-image-slider/0.1/amp-image-slider.css
@@ -95,7 +95,8 @@ amp-image-slider amp-img > img {
bottom: 0;
z-index: 2;
display: flex;
- align-items: center;
+ flex-direction: column;
+ justify-content: center;
transition: opacity 0.3s;
}
@@ -103,21 +104,21 @@ amp-image-slider amp-img > img {
opacity: 0;
}
-.amp-image-slider-hint-left-arrow {
- background-size: 32px 16px;
- width: 32px;
+.amp-image-slider-hint-left {
+ display: inline-block;
+ background-size: 56px 16px;
+ width: 56px;
height: 16px;
- margin-right: 15px !important;
- background-image: url('data:image/svg+xml;charset=utf-8,');
+ background-image: url('data:image/svg+xml;charset=utf-8,');
filter: drop-shadow(3px 3px 4px black);
}
-.amp-image-slider-hint-right-arrow {
- background-size: 32px 16px;
- width: 32px;
+.amp-image-slider-hint-right {
+ display: inline-block;
+ background-size: 56px 16px;
+ width: 56px;
height: 16px;
- margin-left: 15px !important;
- background-image: url('data:image/svg+xml;charset=utf-8,');
+ background-image: url('data:image/svg+xml;charset=utf-8,');
filter: drop-shadow(3px 3px 4px black);
}
diff --git a/extensions/amp-image-slider/0.1/amp-image-slider.js b/extensions/amp-image-slider/0.1/amp-image-slider.js
index 4365972689966..a5ac88c832442 100644
--- a/extensions/amp-image-slider/0.1/amp-image-slider.js
+++ b/extensions/amp-image-slider/0.1/amp-image-slider.js
@@ -18,7 +18,6 @@ import {ActionTrust} from '../../../src/action-constants';
import {CSS} from '../../../build/amp-image-slider-0.1.css';
import {CommonSignals} from '../../../src/common-signals';
import {Gestures} from '../../../src/gesture';
-import {Services} from '../../../src/services';
import {SwipeXRecognizer} from '../../../src/gesture-recognizers';
import {clamp} from '../../../src/utils/math';
import {dev, user} from '../../../src/log';
@@ -70,6 +69,10 @@ export class AmpImageSlider extends AMP.BaseElement {
/** @private {Element|null} */
this.hint_ = null;
+ /** @private {Element|null} */
+ this.hintLeftArrow_ = null;
+ /** @private {Element|null} */
+ this.hintRightArrow_ = null;
/** @private {UnlistenDef|null} */
this.unlistenMouseDown_ = null;
@@ -86,15 +89,8 @@ export class AmpImageSlider extends AMP.BaseElement {
(Number(this.element.getAttribute('step-size')) || 0.1) : 0.1;
/** @private {boolean} */
- this.disableHint_ = this.element.hasAttribute('disable-hint');
- /** @private {number} */
- this.hintReappearInterval_ = 10000;
- /** @private {boolean} */
- this.shouldHintReappear_ = false;
- /** @private {number|null} */
- this.hintTimeoutHandle_ = null;
- /** @private {boolean} */
- this.isHintHidden_ = false;
+ this.shouldHintReappear_ =
+ !this.element.hasAttribute('disable-hint-reappear');
/** @private {boolean} */
this.gestureDisabled_ = false; // for now, will be set later
@@ -162,7 +158,9 @@ export class AmpImageSlider extends AMP.BaseElement {
const percent = args['percent'];
user().assertNumber(percent,
'value to seek to must be a number');
- this.updatePositions_(percent);
+ this.mutateElement(() => {
+ this.updatePositions_(percent);
+ });
}
}
}, ActionTrust.LOW);
@@ -201,10 +199,6 @@ export class AmpImageSlider extends AMP.BaseElement {
if (!this.gestureDisabled_) {
return;
}
- // Show hint if needed
- if (!this.disableHint_) {
- this.animateShowHint_();
- }
this.registerEvents_();
this.gestureDisabled_ = false;
}
@@ -217,13 +211,6 @@ export class AmpImageSlider extends AMP.BaseElement {
if (this.gestureDisabled_) {
return;
}
- // Hide hint if needed
- if (!this.disableHint_) {
- // need to clear timeout handle inside
- // thus not directly calling animateHideHint_
- this.resetHintReappear_(true); // no restart
- this.isHintHidden_ = true;
- }
this.unregisterEvents_();
this.gestureDisabled_ = true;
}
@@ -284,29 +271,18 @@ export class AmpImageSlider extends AMP.BaseElement {
* @private
*/
buildHint_() {
- if (this.disableHint_) {
- return;
- }
-
this.hint_ = this.doc_.createElement('div');
- if (this.element.hasAttribute('hint-reappear')) {
- this.shouldHintReappear_ = true;
- }
-
- if (this.element.hasAttribute('hint-reappear-interval')) {
- this.hintReappearInterval_ =
- Number(this.element.getAttribute('hint-reappear-interval')) ||
- this.hintReappearInterval_;
- }
+ const hintArrowWrapper = this.doc_.createElement('div');
+ this.hintLeftArrow_ = htmlFor(this.doc_)
+ ``;
+ this.hintRightArrow_ = htmlFor(this.doc_)
+ ``;
- const leftHintIcon = htmlFor(this.doc_)
- ``;
- const rightHintIcon = htmlFor(this.doc_)
- ``;
+ hintArrowWrapper.appendChild(this.hintLeftArrow_);
+ hintArrowWrapper.appendChild(this.hintRightArrow_);
- this.hint_.appendChild(leftHintIcon);
- this.hint_.appendChild(rightHintIcon);
+ this.hint_.appendChild(hintArrowWrapper);
this.hint_.classList.add('i-amphtml-image-slider-hint');
this.hint_.classList.add('i-amphtml-image-slider-push-left');
this.bar_.appendChild(this.hint_);
@@ -370,26 +346,18 @@ export class AmpImageSlider extends AMP.BaseElement {
this.gestures_ = Gestures.get(this.element);
this.gestures_.onGesture(SwipeXRecognizer, e => {
- // We need the initial offset, yet gesture event seems not providing
if (e.data.first) {
// Disable hint reappearance timeout if needed
- this.resetHintReappear_(true);
+ this.animateHideHint_();
}
this.pointerMoveX_(
e.data.startX + e.data.deltaX);
- if (e.data.last) {
- // Reset hint reappearance timeout if needed
- this.resetHintReappear_(!this.shouldHintReappear_);
- }
});
this.gestures_.onPointerDown(e => {
// Ensure touchstart changes slider position
this.pointerMoveX_(e.touches[0].pageX, true);
- // Use !this.shouldHintReappear_ here
- // It is possible that after onPointerDown
- // SwipeXRecognizer callback is not triggered
- this.resetHintReappear_(!this.shouldHintReappear_);
+ this.animateHideHint_();
});
}
@@ -405,45 +373,14 @@ export class AmpImageSlider extends AMP.BaseElement {
this.gestures_ = null;
}
- /**
- * Reset interval when the hint would reappear
- * Call this when an user interaction is done
- * Specify opt_noRestart to true if no intent to start a timeout for
- * showing hint again.
- * @param {boolean=} opt_noRestart
- * @private
- */
- resetHintReappear_(opt_noRestart) {
- if (this.disableHint_) {
- return;
- }
-
- if (!this.isHintHidden_) {
- this.animateHideHint_();
- }
-
- if (this.hintTimeoutHandle_ !== null) {
- Services.timerFor(this.win).cancel(this.hintTimeoutHandle_);
- }
-
- if (opt_noRestart === true) {
- this.hintTimeoutHandle_ = null;
- return;
- }
-
- // Use timer instead of default setTimeout
- this.hintTimeoutHandle_ = Services.timerFor(this.win).delay(() => {
- this.animateShowHint_();
- }, this.hintReappearInterval_);
- }
-
/**
* Show hint with animation
* @private
*/
animateShowHint_() {
- this.hint_.classList.remove('i-amphtml-image-slider-hint-hidden');
- this.isHintHidden_ = false;
+ this.mutateElement(() => {
+ this.hint_.classList.remove('i-amphtml-image-slider-hint-hidden');
+ });
}
/**
@@ -451,8 +388,9 @@ export class AmpImageSlider extends AMP.BaseElement {
* @private
*/
animateHideHint_() {
- this.hint_.classList.add('i-amphtml-image-slider-hint-hidden');
- this.isHintHidden_ = true;
+ this.mutateElement(() => {
+ this.hint_.classList.add('i-amphtml-image-slider-hint-hidden');
+ });
}
/**
@@ -474,7 +412,7 @@ export class AmpImageSlider extends AMP.BaseElement {
this.unlistenMouseUp_ =
listen(this.win, 'mouseup', this.onMouseUp_.bind(this));
- this.resetHintReappear_(true);
+ this.animateHideHint_();
}
/**
@@ -496,8 +434,6 @@ export class AmpImageSlider extends AMP.BaseElement {
e.preventDefault();
this.unlisten_(this.unlistenMouseMove_);
this.unlisten_(this.unlistenMouseUp_);
-
- this.resetHintReappear_(!this.shouldHintReappear_);
}
/**
@@ -511,7 +447,7 @@ export class AmpImageSlider extends AMP.BaseElement {
return;
}
- this.resetHintReappear_(!this.shouldHintReappear_);
+ this.animateHideHint_();
switch (e.key.toLowerCase()) {
case 'arrowleft':
@@ -770,6 +706,15 @@ export class AmpImageSlider extends AMP.BaseElement {
resumeCallback() {
this.registerEvents_();
}
+
+ /** @override */
+ viewportCallback(inViewport) {
+ // Show hint if back into viewport and user does not explicitly
+ // disable this
+ if (inViewport && this.shouldHintReappear_) {
+ this.animateShowHint_();
+ }
+ }
}
AMP.extension('amp-image-slider', '0.1', AMP => {
diff --git a/extensions/amp-image-slider/0.1/test/integration/test-amp-image-slider.js b/extensions/amp-image-slider/0.1/test/integration/test-amp-image-slider.js
index 16314e46a7581..98f2ea553f2ba 100644
--- a/extensions/amp-image-slider/0.1/test/integration/test-amp-image-slider.js
+++ b/extensions/amp-image-slider/0.1/test/integration/test-amp-image-slider.js
@@ -28,9 +28,17 @@ config.run('amp-image-slider', function() {
BEFORE
AFTER
+
+
HUGE PADDING
`;
const css = `
+ #slider .amp-image-slider-hint-left {
+ width: 64px;
+ height: 32px;
+ background-width: 64px;
+ background-height: 32px;
+ }
.label {
color: white;
border: 4px solid white;
@@ -38,6 +46,9 @@ config.run('amp-image-slider', function() {
font-family: Arial, Helvetica, sans-serif;
box-shadow: 2px 2px 27px 5px rgba(0,0,0,0.75);
}
+ .para {
+ height: 10000px;
+ }
`;
const experiments = ['amp-image-slider'];
@@ -96,6 +107,10 @@ config.run('amp-image-slider', function() {
return event;
}
+ function timeout(win, ms) {
+ return new Promise(resolve => win.setTimeout(resolve, ms));
+ }
+
function createKeyDownEvent(key) {
return new KeyboardEvent('keydown', {
key,
@@ -116,7 +131,7 @@ config.run('amp-image-slider', function() {
// A bunch of expects to check if the slider has slided to
// where we intended
function expectByBarLeftPos(leftPos) {
- slider = doc.querySelector('amp-image-slider');
+ slider = doc.querySelector('#slider');
bar_ = slider.querySelector('.i-amphtml-image-slider-bar');
container_ =
slider.querySelector('.i-amphtml-image-slider-container');
@@ -130,19 +145,22 @@ config.run('amp-image-slider', function() {
rightLabel_ = slider.querySelectorAll(
'.i-amphtml-image-slider-label-wrapper')[1];
- expect(bar_.getBoundingClientRect().left)
- .to.equal(leftPos);
- expect(rightMask_.getBoundingClientRect().left)
- .to.equal(leftPos);
+ const roundedLeftPos = Math.round(leftPos);
+ const roundedRectLeft = Math.round(rect.left);
+
+ expect(Math.round(bar_.getBoundingClientRect().left))
+ .to.equal(roundedLeftPos);
+ expect(Math.round(rightMask_.getBoundingClientRect().left))
+ .to.equal(roundedLeftPos);
// amp-imgs should stay where they are
- expect(leftAmpImage_.getBoundingClientRect().left)
- .to.equal(rect.left);
- expect(rightAmpImage_.getBoundingClientRect().left)
- .to.equal(rect.left);
- expect(leftLabel_.getBoundingClientRect().left)
- .to.equal(rect.left);
- expect(rightLabel_.getBoundingClientRect().left)
- .to.equal(rect.left);
+ expect(Math.round(leftAmpImage_.getBoundingClientRect().left))
+ .to.equal(roundedRectLeft);
+ expect(Math.round(rightAmpImage_.getBoundingClientRect().left))
+ .to.equal(roundedRectLeft);
+ expect(Math.round(leftLabel_.getBoundingClientRect().left))
+ .to.equal(roundedRectLeft);
+ expect(Math.round(rightLabel_.getBoundingClientRect().left))
+ .to.equal(roundedRectLeft);
}
it('should construct', () => {
@@ -242,10 +260,6 @@ config.run('amp-image-slider', function() {
});
});
- function timeout(win, ms) {
- return new Promise(resolve => win.setTimeout(resolve, ms));
- }
-
it('should follow mouse drag', () => {
let slider;
let mouseDownEvent, mouseMoveEvent, mouseUpEvent;
@@ -430,20 +444,20 @@ config.run('amp-image-slider', function() {
it('should follow keyboard buttons', () => {
let slider;
let keyDownEvent;
- let centerPos, Pos40Percent;
+ let centerPos, pos40Percent;
return prep()
.then(() => {
slider = doc.querySelector('amp-image-slider');
slider.focus();
centerPos = rect.left + 0.5 * rect.width;
- Pos40Percent = rect.left + 0.4 * rect.width;
+ pos40Percent = rect.left + 0.4 * rect.width;
keyDownEvent = createKeyDownEvent('ArrowLeft');
slider.dispatchEvent(keyDownEvent);
return timeout(env.win, 500);
})
.then(() => {
- expectByBarLeftPos(Pos40Percent);
+ expectByBarLeftPos(pos40Percent);
keyDownEvent = createKeyDownEvent('ArrowRight');
slider.dispatchEvent(keyDownEvent);
diff --git a/extensions/amp-image-slider/amp-image-slider.md b/extensions/amp-image-slider/amp-image-slider.md
index dbead97bb05aa..30f8072a9bd52 100644
--- a/extensions/amp-image-slider/amp-image-slider.md
+++ b/extensions/amp-image-slider/amp-image-slider.md
@@ -59,3 +59,5 @@ The slider could also take 2 optional `div`s for labels, which are used to add e
By default, once the slider is loaded, the compared images will be separated by a vertical bar, with helpful arrow hints displayed at its center. User could press mouse button down or touch to move the slider to the position of the pointer, and could then move the pointer to drag the slider bar left and right. For the left image, only part that is left to the bar is displayed; similar for the right image to only display the right portion.
If we specify `tabindex` on `amp-image-slider`, users are also allowed to move the slider using keyboard. Pressing down left and right arrow moves the slider bar one step towards the corresponding direction. Pressing Home would bring the slider to the center, PageUp and PageDown to the left and right end of the slider body.
+
+The hints that shows at the center of the vertical bar will disappear once user starts interacting with the slider (such as clicking the mouse button, touch the slider, and pressing keys to move the slider). The hints would reappear if it then leaves the viewport and goes back in again. To stop such reappearing behavior, add the `disable-hint-reappear` attribute to the image slider.
diff --git a/test/manual/amp-image-slider.amp.html b/test/manual/amp-image-slider.amp.html
index 9919ea48083e0..8d63abc1d6789 100644
--- a/test/manual/amp-image-slider.amp.html
+++ b/test/manual/amp-image-slider.amp.html
@@ -100,7 +100,7 @@
P0 TESTS
AFTER
-
Labels with positioning rules specified
+
Labels with positioning rules specified (both should be at the center)