Turn this into experiment

ampproject · Aug 2, 2018 · 193f4b1 · 193f4b1
1 parent 08af034
commit 193f4b1
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 21 deletions.
diff --git a/build-system/global-configs/canary-config.json b/build-system/global-configs/canary-config.json
@@ -33,6 +33,7 @@
   "amp-live-list-sorting": 1,
   "amp-sidebar toolbar": 1,
   "svg-in-mustache": 1,
+  "amp-carousel-scroll-snap": 1,
   "amp-consent": 1,
   "amp-img-native-srcset": 1,
   "amp-story-v1": 1,

diff --git a/extensions/amp-carousel/0.1/amp-carousel.css b/extensions/amp-carousel/0.1/amp-carousel.css
@@ -131,7 +131,7 @@ amp-carousel .amp-carousel-button.amp-disabled {
   height: 100% !important;
   justify-content: center !important;
   position: relative !important;
-  scroll-snap-align: center !important;
+  scroll-snap-align: start !important;
   width: 100% !important;
 }
 
@@ -152,7 +152,7 @@ amp-carousel .amp-carousel-button.amp-disabled {
   flex: 0 0 1px !important;
   height: 100% !important;
   position: relative !important;
-  scroll-snap-align: center !important;
+  scroll-snap-align: start !important;
   width: 1px !important;
 }
 

diff --git a/extensions/amp-carousel/0.1/slidescroll.js b/extensions/amp-carousel/0.1/slidescroll.js
@@ -23,10 +23,10 @@ import {createCustomEvent} from '../../../src/event-helper';
 import {dev} from '../../../src/log';
 import {dict} from '../../../src/utils/object';
 import {getStyle, setStyle} from '../../../src/style';
+import {isExperimentOn} from '../../../src/experiments';
 import {isFiniteNumber} from '../../../src/types';
 import {isLayoutSizeDefined} from '../../../src/layout';
 import {numeric} from '../../../src/transition';
-import {startsWith} from '../../../src/string';
 import {triggerAnalyticsEvent} from '../../../src/analytics';
 
 /** @const {string} */
@@ -123,8 +123,7 @@ export class AmpSlideScroll extends BaseSlides {
     this.action_ = null;
 
     /** @private {boolean} */
-    this.shouldDisableCssSnap_ = startsWith(
-        Services.platformFor(this.win).getIosVersionString(), '10.3');
+    this.shouldDisableCssSnap_ = isExperimentOn(this.win, 'amp-carousel-scroll-snap');
   }
 
   /** @override */

diff --git a/extensions/amp-carousel/0.1/test/test-slidescroll.js b/extensions/amp-carousel/0.1/test/test-slidescroll.js
@@ -94,22 +94,6 @@ describes.realWin('SlideScroll', {
     });
   });
 
-  it('should create start/end markers when scroll-snap is available', () => {
-    return getAmpSlideScroll().then(ampSlideScroll => {
-      const impl = ampSlideScroll.implementation_;
-      ampSlideScroll.style['scrollSnapType'] = '';
-      ampSlideScroll.style['webkitScrollSnapType'] = '';
-      ampSlideScroll.style['msScrollSnapType'] = '';
-      impl.buildCarousel();
-      expect(
-          ampSlideScroll.getElementsByClassName(
-              'i-amphtml-carousel-start-marker').length).to.be.at.least(1);
-      expect(
-          ampSlideScroll.getElementsByClassName(
-              'i-amphtml-carousel-end-marker').length).to.be.at.least(1);
-    });
-  });
-
   it('should go to the correct slide on button click', () => {
     return getAmpSlideScroll().then(ampSlideScroll => {
       const impl = ampSlideScroll.implementation_;

diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js
@@ -321,6 +321,12 @@ const EXPERIMENTS = [
     spec: 'https://github.com/ampproject/amphtml/pull/17229',
     cleanupIssue: 'https://github.com/ampproject/amphtml/issues/17230',
   },
+  {
+    id: 'amp-carousel-scroll-snap',
+    name: 'Enables scroll snap on carousel across all browsers/OSes',
+    cleanupIssue: 'https://github.com/ampproject/amphtml/issues/16508',
+  },
+
 ];
 
 if (getMode().localDev) {