Reader: fix scroll depth tracking

client/blocks/reader-full-post/index.jsx

@@ -77,6 +77,7 @@ import isSiteWPForTeams from 'calypso/state/selectors/is-site-wpforteams';
 import { disableAppBanner, enableAppBanner } from 'calypso/state/ui/actions';
 import ReaderFullPostHeader from './header';
 import ReaderFullPostContentPlaceholder from './placeholders/content';
+import ScrollTracker from './scroll-tracker';
 import ReaderFullPostUnavailable from './unavailable';
 
 import './style.scss';
@@ -101,8 +102,6 @@ export class FullPostView extends Component {
 
 	state = {
 		isSuggestedFollowsModalOpen: false,
-		maxScrollDepth: 0, // Track the maximum scroll depth achieved
-		hasCompleted: false, // Track whether the user completed the post
 	};
 
 	openSuggestedFollowsModal = ( followClicked ) => {
@@ -113,6 +112,7 @@ export class FullPostView extends Component {
 	};
 
 	componentDidMount() {
+		this.scrollTracker = new ScrollTracker();
 		// Send page view
 		this.hasSentPageView = false;
 		this.hasLoaded = false;
@@ -141,8 +141,8 @@ export class FullPostView extends Component {
 			document.querySelector( '#primary > div > div.recent-feed > section' ) || // for Recent Feed in Dataview
 			document.querySelector( '#primary > div > div' ); // for Recent Feed in Stream
 		if ( scrollableContainer ) {
-			scrollableContainer.addEventListener( 'scroll', this.setScrollDepth );
-			this.scrollableContainer = scrollableContainer; // Save reference for cleanup
+			this.scrollableContainer = scrollableContainer;
+			this.scrollTracker.setContainer( scrollableContainer );
 			this.resetScroll();
 		}
 	}
@@ -197,9 +197,9 @@ export class FullPostView extends Component {
 		document.removeEventListener( 'keydown', this.handleKeydown, true );
 		document.removeEventListener( 'visibilitychange', this.handleVisibilityChange );
 
-		if ( this.scrollableContainer ) {
-			this.scrollableContainer.removeEventListener( 'scroll', this.setScrollDepth );
-		}
+		// Track scroll depth and remove related instruments
+		this.trackScrollDepth( this.props.post );
+		this.scrollTracker.cleanup();
 		this.clearResetScrollTimeout();
 	}
 
@@ -280,51 +280,37 @@ export class FullPostView extends Component {
 	resetScroll = () => {
 		this.clearResetScrollTimeout();
 		this.resetScrollTimeout = setTimeout( () => {
-			if ( this.scrollableContainer ) {
-				this.scrollableContainer.scrollTo( {
-					top: 0,
-					left: 0,
-					behavior: 'instant',
-				} );
-			}
-			this.setState( { maxScrollDepth: 0, hasCompleted: false } );
+			this.scrollableContainer.scrollTo( {
+				top: 0,
+				left: 0,
+				behavior: 'instant',
+			} );
+			this.scrollTracker.resetMaxScrollDepth();
 		}, 0 ); // Defer until after the DOM update
 	};
 
-	setScrollDepth = () => {
-		if ( this.scrollableContainer ) {
-			const scrollTop = this.scrollableContainer.scrollTop;
-			const scrollHeight = this.scrollableContainer.scrollHeight;
-			const clientHeight = this.scrollableContainer.clientHeight;
-			const scrollDepth = ( scrollTop / ( scrollHeight - clientHeight ) ) * 100;
-			this.setState( ( prevState ) => ( {
-				maxScrollDepth: Math.max( prevState.maxScrollDepth, scrollDepth ) || 0,
-				hasCompleted: prevState.hasCompleted || scrollDepth >= 90,
-			} ) );
-		}
-	};
-
 	trackScrollDepth = ( post = null ) => {
-		const { maxScrollDepth } = this.state;
 		if ( ! post ) {
 			post = this.props.post;
 		}
 
 		if ( this.scrollableContainer && post.ID ) {
-			const roundedDepth = Math.round( maxScrollDepth * 100 ) / 100;
+			const maxScrollDepth = this.scrollTracker.getMaxScrollDepthAsPercentage();
 			recordTrackForPost( 'calypso_reader_article_scroll_depth', post, {
 				context: 'full-post',
-				scroll_depth: roundedDepth,
+				scroll_depth: maxScrollDepth,
 			} );
 		}
 	};
 
 	trackExitBeforeCompletion = ( post = null ) => {
-		const { hasCompleted, maxScrollDepth } = this.state;
 		if ( ! post ) {
 			post = this.props.post;
 		}
 
+		const maxScrollDepth = this.scrollTracker.getMaxScrollDepthAsPercentage();
+		const hasCompleted = maxScrollDepth >= 90; // User has read 90% of the post
+
 		if ( this.scrollableContainer && post.ID && ! hasCompleted ) {
 			recordTrackForPost( 'calypso_reader_article_exit_before_completion', post, {
 				context: 'full-post',

client/blocks/reader-full-post/scroll-tracker.ts

@@ -0,0 +1,79 @@
+/**
+ * Tracks scroll depth for a container element.
+ */
+export default class ScrollTracker {
+	private container: HTMLElement | null = null;
+	private maxScrollDepth: number = 0;
+
+	private handleScroll = (): void => {
+		if ( ! this.container ) {
+			return;
+		}
+
+		const scrollTop = this.container.scrollTop ?? 0;
+		const scrollHeight = this.container.scrollHeight ?? 0;
+		const clientHeight = this.container.clientHeight ?? 0;
+
+		const denominator = scrollHeight - clientHeight;
+		const scrollDepth = denominator <= 0 ? 0 : scrollTop / denominator;
+
+		this.maxScrollDepth = calcAndClampMaxValue( scrollDepth, this.maxScrollDepth );
+	};
+
+	/**
+	 * Sets the container element to track scrolling on.
+	 * Removes scroll listener from previous container if it exists.
+	 * @param container - The HTML element to track scrolling on, or null to stop tracking
+	 */
+	public setContainer( container: HTMLElement | null ): void {
+		this.cleanup();
+		this.resetMaxScrollDepth();
+		this.container = container;
+		if ( container ) {
+			container.addEventListener( 'scroll', this.handleScroll );
+		}
+	}
+
+	/**
+	 * Gets the maximum scroll depth reached as a decimal between 0 and 1.
+	 * @returns A number between 0 and 1 representing the maximum scroll depth
+	 */
+	public getMaxScrollDepth(): number {
+		return this.maxScrollDepth;
+	}
+
+	/**
+	 * Gets the maximum scroll depth reached as a percentage between 0 and 100.
+	 * @returns A rounded number between 0 and 100 representing the maximum scroll depth percentage
+	 */
+	public getMaxScrollDepthAsPercentage(): number {
+		return Math.round( this.maxScrollDepth * 100 );
+	}
+
+	/**
+	 * Resets the maximum scroll depth back to 0.
+	 */
+	public resetMaxScrollDepth = (): void => {
+		this.maxScrollDepth = 0;
+	};
+
+	/**
+	 * Removes scroll event listener from container.
+	 * Should be called when tracking is no longer needed.
+	 */
+	public cleanup(): void {
+		if ( this.container ) {
+			this.container.removeEventListener( 'scroll', this.handleScroll );
+		}
+	}
+}
+
+/**
+ * Calculates the maximum value between two numbers and clamps the result between 0 and 1.
+ * @param valueA - First number to compare
+ * @param valueB - Second number to compare
+ * @returns A number between 0 and 1 representing the maximum value between valueA and valueB
+ */
+function calcAndClampMaxValue( valueA: number, valueB: number ): number {
+	return Math.min( 1, Math.max( 0, valueA, valueB ) );
+}

client/blocks/reader-full-post/test/scroll-tracker.ts

@@ -0,0 +1,130 @@
+/**
+ * @jest-environment jsdom
+ */
+import ScrollTracker from '../scroll-tracker';
+
+describe( 'ScrollTracker', () => {
+	let scrollTracker;
+	let container;
+
+	beforeEach( () => {
+		scrollTracker = new ScrollTracker();
+		container = document.createElement( 'div' );
+		// Setup scrollable container
+		Object.defineProperties( container, {
+			scrollTop: { value: 0, configurable: true },
+			scrollHeight: { value: 1000, configurable: true },
+			clientHeight: { value: 500, configurable: true },
+		} );
+	} );
+
+	afterEach( () => {
+		scrollTracker.cleanup();
+	} );
+
+	describe( 'getMaxScrollDepth()', () => {
+		it( 'should return 0 when no scrolling has occurred', () => {
+			expect( scrollTracker.getMaxScrollDepth() ).toBe( 0 );
+		} );
+
+		it( 'should return correct depth when scrolled', () => {
+			scrollTracker.setContainer( container );
+			Object.defineProperty( container, 'scrollTop', { value: 250 } );
+			container.dispatchEvent( new Event( 'scroll' ) );
+			expect( scrollTracker.getMaxScrollDepth() ).toBe( 0.5 );
+		} );
+
+		it( 'should return the highest depth reached', () => {
+			scrollTracker.setContainer( container );
+
+			Object.defineProperty( container, 'scrollTop', { value: 375 } );
+			container.dispatchEvent( new Event( 'scroll' ) );
+
+			Object.defineProperty( container, 'scrollTop', { value: 250 } );
+			container.dispatchEvent( new Event( 'scroll' ) );
+
+			expect( scrollTracker.getMaxScrollDepth() ).toBe( 0.75 );
+		} );
+
+		it( 'should reset when container changes', () => {
+			scrollTracker.setContainer( container );
+			Object.defineProperty( container, 'scrollTop', { value: 250 } );
+			container.dispatchEvent( new Event( 'scroll' ) );
+
+			const newContainer = document.createElement( 'div' );
+			Object.defineProperties( newContainer, {
+				scrollTop: { value: 0, configurable: true },
+				scrollHeight: { value: 1000, configurable: true },
+				clientHeight: { value: 500, configurable: true },
+			} );
+			scrollTracker.setContainer( newContainer );
+
+			expect( scrollTracker.getMaxScrollDepth() ).toBe( 0 );
+		} );
+	} );
+
+	describe( 'getMaxScrollDepthAsPercentage()', () => {
+		it( 'should return 0 when no scrolling has occurred', () => {
+			expect( scrollTracker.getMaxScrollDepthAsPercentage() ).toBe( 0 );
+		} );
+
+		it( 'should return the correct percentage when scrolled', () => {
+			scrollTracker.setContainer( container );
+			Object.defineProperty( container, 'scrollTop', { value: 250 } );
+			container.dispatchEvent( new Event( 'scroll' ) );
+			expect( scrollTracker.getMaxScrollDepthAsPercentage() ).toBe( 50 );
+		} );
+
+		it( 'should return the highest percentage reached', () => {
+			scrollTracker.setContainer( container );
+
+			Object.defineProperty( container, 'scrollTop', { value: 375 } );
+			container.dispatchEvent( new Event( 'scroll' ) );
+
+			Object.defineProperty( container, 'scrollTop', { value: 250 } );
+			container.dispatchEvent( new Event( 'scroll' ) );
+
+			expect( scrollTracker.getMaxScrollDepthAsPercentage() ).toBe( 75 );
+		} );
+	} );
+
+	describe( 'setContainer()', () => {
+		it( 'should track scroll events on the new container', () => {
+			scrollTracker.setContainer( container );
+			Object.defineProperty( container, 'scrollTop', { value: 250 } );
+			container.dispatchEvent( new Event( 'scroll' ) );
+			expect( scrollTracker.getMaxScrollDepthAsPercentage() ).toBe( 50 );
+		} );
+
+		it( 'should stop tracking previous container when setting new one', () => {
+			const oldContainer = document.createElement( 'div' );
+			Object.defineProperties( oldContainer, {
+				scrollTop: { value: 0, configurable: true },
+				scrollHeight: { value: 1000, configurable: true },
+				clientHeight: { value: 500, configurable: true },
+			} );
+
+			scrollTracker.setContainer( oldContainer );
+			Object.defineProperty( oldContainer, 'scrollTop', { value: 250 } );
+			oldContainer.dispatchEvent( new Event( 'scroll' ) );
+
+			scrollTracker.setContainer( container );
+			Object.defineProperty( oldContainer, 'scrollTop', { value: 375 } );
+			oldContainer.dispatchEvent( new Event( 'scroll' ) );
+
+			expect( scrollTracker.getMaxScrollDepthAsPercentage() ).toBe( 0 );
+		} );
+	} );
+
+	describe( 'cleanup()', () => {
+		it( 'should stop tracking scroll events', () => {
+			scrollTracker.setContainer( container );
+			scrollTracker.cleanup();
+
+			Object.defineProperty( container, 'scrollTop', { value: 250 } );
+			container.dispatchEvent( new Event( 'scroll' ) );
+
+			expect( scrollTracker.getMaxScrollDepthAsPercentage() ).toBe( 0 );
+		} );
+	} );
+} );

client/reader/recent/style.scss

@@ -18,6 +18,7 @@ body.is-reader-full-post {
 	);
 
 	height: $feed-content-height !important; // Using !important here to avoid having a lengthy selector.
+	overflow-y: hidden !important; // Keeps the scrolling container consistent between one and two column layouts.
 
 	@media ( min-width: $break-wide ) {
 		display: flex;
@@ -200,6 +201,7 @@ body.is-reader-full-post {
 		@extend %column-shared;
 		display: none;
 		overflow-y: auto;
+		height: 100%;
 
 		&.overlay {
 			display: block;