Skip to content

Commit

Permalink
bug fix: slides smooth scroll animation (#133)
Browse files Browse the repository at this point in the history
* fixed selector for slides container

* rm tabindex -1 from slides:

seems to prevent smooth scroll from functioning correctly?

* pass html id value to gsap.to options:

seems to work better than using .slide.active?

* scroll to active slide on init:
to make sure the slides navigation is always starting at slide 0

* better naming for method params

* rm console.logs

* upgraded dep gsap to 3.12.2

* added back tabindex=-1 on slides

* focus active slide on scroll complete

* rm redux logger in start script

* clean-up SlidesContainer

* fixed tests for SlidesContainer

* improved test in SlidesContainer:

- test: handleSlidesUpdate responds to state.slides changes
- check for value of inert & aria-hidden attributes on all slides

* added test for scroll complete

* improved test "checks prefers-reduced-motion media query"

* use gsap.fromTo for slide scroll

- to avoid scrolling from first slide to third slide after
  the user searches their address. Fixes a side effect for
  bugfix for #131

* fixed test for SlidesContainer.scrollToActiveSlide

* cleaned up activeSlide and activeSlideIndex setters logic
  • Loading branch information
clhenrick authored Jan 1, 2024
1 parent 4e4ae49 commit b27197c
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 31 deletions.
2 changes: 1 addition & 1 deletion app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
"cross-fetch": "^3.1.5",
"d3-geo": "^2.0.1",
"d3-tile": "^1.0.0",
"gsap": "^3.6.0",
"gsap": "^3.12.2",
"handlebars": "4.7.7",
"innersvg-polyfill": "^0.0.5",
"lodash.throttle": "^4.1.1",
Expand Down
57 changes: 42 additions & 15 deletions app/src/components/slidesContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import { ScrollToPlugin } from "gsap/ScrollToPlugin";
import { Component } from "./_componentBase";
import { observeStore } from "../store";

// NOTE: this is required for GSAP plugins to work with module bundlers
gsap.registerPlugin(ScrollToPlugin);

const SCROLL_DURATION_SECONDS = 0.65;
export const SCROLL_DURATION_SECONDS = 0.65;

export class SlidesContainer extends Component {
constructor(props) {
Expand All @@ -17,34 +18,57 @@ export class SlidesContainer extends Component {

this.slides = [...this.element.querySelectorAll(".slide")];
this.prefersReducedMotion = false;
this.activeSlide = this.store.getState().slides.curIndex;
this.activeSlideIndex = this.store.getState().slides.curIndex;
this.previousSlideIndex = undefined;

this.handleSlidesUpdate = this.handleSlidesUpdate.bind(this);
this.scrollToActiveSlide = this.scrollToActiveSlide.bind(this);
this.handleMotionQuery = this.handleMotionQuery.bind(this);
this.handleScrollComplete = this.handleScrollComplete.bind(this);

this.handleMotionQuery();
this.unsubscribe = observeStore(
this.store,
(state) => state.slides,
this.handleSlidesUpdate
);
this.scrollToActiveSlide();
}

handleSlidesUpdate() {
const { slides } = this.store.getState();
if (slides.curIndex !== this.activeSlideIdx) {
this.activeSlide = slides.curIndex;
if (slides.curIndex !== this.activeSlideIndex) {
this.previousSlideIndex = this.activeSlideIndex;
this.activeSlideIndex = slides.curIndex;
this.scrollToActiveSlide();
}
}

scrollToActiveSlide() {
gsap.to(this.element, {
duration: this.prefersReducedMotion ? 0 : SCROLL_DURATION_SECONDS,
scrollTo: ".slide.active",
const id = `#slide-${this.activeSlideIndex + 1}`;
const duration = this.prefersReducedMotion ? 0 : SCROLL_DURATION_SECONDS;
const toOptions = {
duration,
scrollTo: id,
ease: "sine.inOut",
});
onComplete: this.handleScrollComplete,
};
// NOTE: we call gsap.fromTo when previousSlideIndex exists to avoid unintentionally scrolling from the first slide.
// this avoids a side effect from a bugfix for issue #131 where the non-active slides are hidden then revealed to prevent an undesirable layout shift from the soft keyboard on touch screen devices.
if (
typeof this.previousSlideIndex === "number" &&
!isNaN(this.previousSlideIndex)
) {
const previousId = `#slide-${this.previousSlideIndex + 1}`;
const fromOptions = { scrollTo: previousId };
gsap.fromTo(this.element, fromOptions, toOptions);
} else {
gsap.to(this.element, toOptions);
}
}

handleScrollComplete() {
this.activeSlide.focus();
}

handleMotionQuery() {
Expand All @@ -54,13 +78,13 @@ export class SlidesContainer extends Component {
}
}

set activeSlide(value) {
/** updates a slide in the document to be the "active" one and all other slides to be "inactive" */
set activeSlide(target) {
this.slides.forEach((slide) => {
if (slide === this.slides[value]) {
if (slide === target) {
slide.classList.add("active");
slide.removeAttribute("inert");
slide.setAttribute("aria-hidden", false);
slide.focus();
} else {
slide.classList.remove("active");
slide.setAttribute("inert", true);
Expand All @@ -69,15 +93,18 @@ export class SlidesContainer extends Component {
});
}

/** returns the slide that currently has a class of "active" */
get activeSlide() {
return this.slides[this.activeSlideIdx];
return this.slides[this.activeSlideIndex];
}

set activeSlideIdx(value) {
this.activeSlide = value;
/** alias for setting activeSlide via an index */
set activeSlideIndex(index) {
this.activeSlide = this.slides[index];
}

get activeSlideIdx() {
/** returns the index of the slide with a class of "active" */
get activeSlideIndex() {
return this.slides.findIndex((slide) => slide.classList.contains("active"));
}
}
57 changes: 47 additions & 10 deletions app/src/components/slidesContainer.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { gsap } from "gsap";
import { SlidesContainer } from "./slidesContainer";
import { SlidesContainer, SCROLL_DURATION_SECONDS } from "./slidesContainer";
import { store, observeStore } from "../store";

jest.mock("../store", () => {
Expand All @@ -25,7 +25,7 @@ const mockMatchMedia = (window.matchMedia = jest.fn(() => ({
})));

describe("SlidesContainer", () => {
const selector = ".slides-container";
const selector = ".slides";
let element;
let slidesContainer;
let spyScrollToActiveSlide;
Expand Down Expand Up @@ -87,36 +87,72 @@ describe("SlidesContainer", () => {
expect(observeStore).toHaveBeenCalledTimes(1);
});

test("scrolls to active slide on init", () => {
expect(spyScrollToActiveSlide).toHaveBeenCalledTimes(1);
});

test("handleSlidesUpdate responds to state.slides changes", () => {
spyScrollToActiveSlide.mockClear();
store.getState.mockImplementationOnce(() => ({
slides: {
curIndex: 5,
},
}));
slidesContainer.handleSlidesUpdate();
const activeSlideIndex = slidesContainer.activeSlideIndex;
const activeSlide = slidesContainer.activeSlide;
const nonActiveSlides = slidesContainer.slides.filter(
(slide) => !slide.classList.contains("active")
);
expect(spyScrollToActiveSlide).toHaveBeenCalledTimes(1);
expect(slidesContainer.activeSlideIdx).toBe(5);
expect(activeSlideIndex).toBe(5);
expect(activeSlide.getAttribute("inert")).toBeNull();
expect(activeSlide.getAttribute("aria-hidden")).toBe("false");
nonActiveSlides.forEach((slide) => {
expect(slide.getAttribute("inert")).toBe("true");
expect(slide.getAttribute("aria-hidden")).toBe("true");
});
});

test("scrollToActiveSlide calls gsap.to", () => {
test("scrollToActiveSlide calls gsap.to and gsap.fromTo with expected params", () => {
gsap.to.mockClear();
slidesContainer.scrollToActiveSlide();
expect(gsap.to).toHaveBeenCalledTimes(1);
expect(gsap.to).toHaveBeenCalledWith(element, {
duration: 0.65,
scrollTo: ".slide.active",
duration: SCROLL_DURATION_SECONDS,
scrollTo: "#slide-1",
ease: "sine.inOut",
onComplete: slidesContainer.handleScrollComplete,
});

slidesContainer.activeSlideIndex = 2;
slidesContainer.previousSlideIndex = 1;
slidesContainer.scrollToActiveSlide();
expect(gsap.fromTo).toHaveBeenCalledTimes(1);
expect(gsap.fromTo).toHaveBeenCalledWith(
element,
{ scrollTo: "#slide-2" },
{
duration: SCROLL_DURATION_SECONDS,
scrollTo: "#slide-3",
ease: "sine.inOut",
onComplete: slidesContainer.handleScrollComplete,
}
);
});

test("when scroll completes, active slide is focused", () => {
slidesContainer.handleScrollComplete();
expect(document.activeElement).toBe(slidesContainer.activeSlide);
});

test("checks prefers-reduced-motion media query", () => {
expect(slidesContainer.prefersReducedMotion).toBe(false);
mockMatchMedia.mockImplementation(() => ({ matches: true }));
slidesContainer = new SlidesContainer({
element,
store,
});
expect(mockMatchMedia).toHaveBeenCalledWith(
"(prefers-reduced-motion: reduce)"
);
expect(slidesContainer.prefersReducedMotion).toBe(true);
});

Expand All @@ -125,8 +161,9 @@ describe("SlidesContainer", () => {
slidesContainer.scrollToActiveSlide();
expect(gsap.to).toHaveBeenCalledWith(slidesContainer.element, {
duration: 0,
scrollTo: ".slide.active",
scrollTo: "#slide-1",
ease: "sine.inOut",
onComplete: slidesContainer.handleScrollComplete,
});
});
});
2 changes: 1 addition & 1 deletion app/src/utils/initApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export default function initApp() {
registry.add(
"slidesContainer",
new SlidesContainer({
element: document.querySelector(".slides-container"),
element: document.querySelector(".slides"),
store,
})
);
Expand Down
8 changes: 4 additions & 4 deletions app/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4886,10 +4886,10 @@ growly@^1.3.0:
resolved "https://registry.yarnpkg.com/growly/-/growly-1.3.0.tgz#f10748cbe76af964b7c96c93c6bcc28af120c081"
integrity sha1-8QdIy+dq+WS3yWyTxrzCivEgwIE=

gsap@^3.6.0:
version "3.6.0"
resolved "https://registry.yarnpkg.com/gsap/-/gsap-3.6.0.tgz#925f25370c698ce0f6ea563522da8f6b5ed21b0a"
integrity sha512-0P3syv1TmYr+A/VZ8UMFzw+s0XoaKSzzDFs8NqkXiJTXI4E/VTi0zRjPgxaPBpiUPPycgRnFjLDe0Tb4dRRf+w==
gsap@^3.12.2:
version "3.12.4"
resolved "https://registry.yarnpkg.com/gsap/-/gsap-3.12.4.tgz#b9383fe16bb14968e2c7db2a7c0e308edf551e7b"
integrity sha512-1ByAq8dD0W4aBZ/JArgaQvc0gyUfkGkP8mgAQa0qZGdpOKlSOhOf+WNXjoLimKaKG3Z4Iu6DKZtnyszqQeyqWQ==

handle-thing@^2.0.0:
version "2.0.1"
Expand Down

0 comments on commit b27197c

Please sign in to comment.