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.

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

AMP SlideScroll Experiment: enable looping. #3680

Merged
merged 9 commits into from Jun 21, 2016

Conversation

camelburrito
Copy link
Contributor

In this PR

  • Support for loops

Yet to come

  • Scroll Support for snap-points
  • Polyfill snapping

Part 4 for - #3465

position: relative;
white-space: nowrap;
position: relative !important;
text-align: center;
Copy link
Contributor

Choose a reason for hiding this comment

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

text-align again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed :)

@camelburrito camelburrito force-pushed the scrollflex-loop branch 2 times, most recently from 634328d to 8c77301 Compare June 21, 2016 20:20
@camelburrito
Copy link
Contributor Author

PTAL

.-amp-carousel-end-marker {
background-color: transparent !important;
display: block !important;
flex: 1 0 1px !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

it should be 0 0 1px, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes and done!

@@ -158,26 +174,32 @@ export class AmpSlideScroll extends BaseCarousel {
// (which is at scrollLeft 0) when slide 0 is requested - for all other
// instances we show the second slide (middle slide at
// scrollLeft = slide's width).
const newScrollLeft = (newIndex == 0) ? 0 : this.slideWidth_;
let newScrollLeft = this.slideWidth_;
if ((!this.hasLooping_ && newIndex == 0) || noOfSlides_ == 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Another formula here, if you'd like it better:

if (showIndexArr[0] == newIndex) {
  newScrollLeft = 0;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again taken care by setting hasLooping = false where there is only one slide.

@dvoytenko
Copy link
Contributor

LGTM

@camelburrito camelburrito merged commit b3098a6 into ampproject:master Jun 21, 2016
@camelburrito camelburrito deleted the scrollflex-loop branch June 21, 2016 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Slidescroll experiment
Completed Pull Requests
Development

Successfully merging this pull request may close these issues.

None yet

2 participants