From 7a435e0511b94e4991dadfbc8b293beb099b1058 Mon Sep 17 00:00:00 2001 From: Pete Miller Date: Thu, 12 Aug 2021 14:29:44 -0700 Subject: [PATCH] Brave News: do not request ad content multiple times for the same ad unit --- .../braveToday/cards/displayAd/index.tsx | 45 ++++---------- .../displayAd/useTriggerOnNearViewport.ts | 58 +++++++++++++++++++ .../default/data/getBraveNewsDisplayAd.ts | 6 +- 3 files changed, 74 insertions(+), 35 deletions(-) create mode 100644 components/brave_new_tab_ui/components/default/braveToday/cards/displayAd/useTriggerOnNearViewport.ts diff --git a/components/brave_new_tab_ui/components/default/braveToday/cards/displayAd/index.tsx b/components/brave_new_tab_ui/components/default/braveToday/cards/displayAd/index.tsx index e5363c07d7b7..52bb06c339be 100644 --- a/components/brave_new_tab_ui/components/default/braveToday/cards/displayAd/index.tsx +++ b/components/brave_new_tab_ui/components/default/braveToday/cards/displayAd/index.tsx @@ -11,6 +11,7 @@ import useScrollIntoView from '../../useScrollIntoView' import { useVisitDisplayAdClickHandler } from '../../useReadArticleClickHandler' import { OnVisitDisplayAd, OnViewedDisplayAd, GetDisplayAdContent } from '../..' import CardImage from '../CardImage' +import useTriggerOnNearViewport from './useTriggerOnNearViewport' import * as Styles from './style' type Props = { @@ -22,14 +23,11 @@ type Props = { export default function CardDisplayAd (props: Props) { // Content is retrieved when the element is close to the viewport - // - undefined = not fetched yet - // - null = no ad available for this unit - // - DisplayAd = ad for this unit const [content, setContent] = React.useState(undefined) const [cardRef] = useScrollIntoView(props.shouldScrollIntoView || false) const onClick = useVisitDisplayAdClickHandler(props.onVisitDisplayAd, content ? { ad: content } : undefined) const innerRef = React.useRef(null) - + // Setup an observer to track amount of time viewed React.useEffect(() => { if (!innerRef.current || !content || !props.onViewedDisplayAd) { return @@ -45,36 +43,17 @@ export default function CardDisplayAd (props: Props) { } }, [innerRef.current, props.onViewedDisplayAd, content?.uuid]) // Ask for and render the ad only when we're scrolled close to it - const contentTrigger = React.useRef(null) - const contentTriggerObserver = React.useRef() - React.useEffect(() => { - // Setup observer on first mount - contentTriggerObserver.current = new IntersectionObserver(async (entries) => { - if (entries.some((entry) => entry.isIntersecting)) { - // Get the ad and display it - const ad = await props.getContent() - // Request may not actually come back with an ad - if (ad) { - setContent(ad) - } - } - }, { - // Trigger ad fetch when the ad unit is 1000px away from the viewport - rootMargin: '0px 0px 1000px 0px' - }) - }) - React.useEffect(() => { - // Observe content trigger when it's appropriate. - // Don't observe (or disconnect current observer) if there is already content - if (content || !contentTrigger.current || !contentTriggerObserver.current) { - return - } - const observer = contentTriggerObserver.current - observer.observe(contentTrigger.current) - return () => { - observer.disconnect() + const handleOnNearViewport = React.useCallback(async () => { + // Get the ad and display it + const ad = await props.getContent() + // Request may not actually come back with an ad + if (ad) { + setContent(ad) } - }, [content, contentTrigger.current, contentTriggerObserver.current]) + }, [props.getContent, setContent]) + const handleOnNearViewportRef = React.useRef(handleOnNearViewport) + handleOnNearViewportRef.current = handleOnNearViewport + const [contentTrigger] = useTriggerOnNearViewport(handleOnNearViewportRef) // Render content trigger if (!content) { // verbose ref type conversion due to https://stackoverflow.com/questions/61102101/cannot-assign-refobjecthtmldivelement-to-refobjecthtmlelement-instance diff --git a/components/brave_new_tab_ui/components/default/braveToday/cards/displayAd/useTriggerOnNearViewport.ts b/components/brave_new_tab_ui/components/default/braveToday/cards/displayAd/useTriggerOnNearViewport.ts new file mode 100644 index 000000000000..27c9509dadd6 --- /dev/null +++ b/components/brave_new_tab_ui/components/default/braveToday/cards/displayAd/useTriggerOnNearViewport.ts @@ -0,0 +1,58 @@ +// Copyright (c) 2021 The Brave Authors. All rights reserved. +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. + +import * as React from 'react' + +let instanceCount = 0 + +export default function useTriggerOnNearViewport (handlerRef: React.MutableRefObject) { + // When we're intersecting for the first time, ask for an Ad + const hasPassedAdRequestThreshold = React.useRef(false) + // Element which will be observed to trigger ad fetch + const contentTrigger = React.useRef(null) + // Observer which will observe elements scroll position to trigger ad fetch + const contentTriggerObserver = React.useRef() + const debugInstanceId = React.useMemo(function () { + return instanceCount++ + }, []) + // Setup ad fetch trigger observer on first mount + React.useEffect(() => { + console.debug('Brave News: creating observer', debugInstanceId) + contentTriggerObserver.current = new IntersectionObserver(async (entries) => { + // Only request an ad the first time it passes threshold + if (!hasPassedAdRequestThreshold.current && entries.some((entry) => entry.isIntersecting)) { + hasPassedAdRequestThreshold.current = true + console.debug('Brave News: asking for an ad', debugInstanceId) + handlerRef.current && handlerRef.current() + } + }, { + // Trigger ad fetch when the ad unit is 1000px away from the viewport + rootMargin: '0px 0px 1000px 0px' + }) + }, []) + // Observe and disconnect from content trigger when it's appropriate + React.useEffect(() => { + if (hasPassedAdRequestThreshold.current || !contentTrigger.current || !contentTriggerObserver.current) { + console.debug('Brave News: not creating trigger', debugInstanceId) + return + } + const observer = contentTriggerObserver.current + console.debug('Brave News: ad fetch trigger observer connected', debugInstanceId) + observer.observe(contentTrigger.current) + return () => { + console.debug('Brave News: ad fetch trigger observer disconnected', debugInstanceId) + observer.disconnect() + } + }, [ + // Disconnect when we have asked for an ad + // (opportunistically at the next render as ref mutations don't cause effects to run) + hasPassedAdRequestThreshold.current, + // Observer or Disconnect when we have a new element (should have caused a render, so this should get run) + contentTrigger.current, + // Observe or Disconnect when we have a new intersection observer (should be at first mount) + contentTriggerObserver.current + ]) + return [contentTrigger] +} diff --git a/components/brave_new_tab_ui/stories/default/data/getBraveNewsDisplayAd.ts b/components/brave_new_tab_ui/stories/default/data/getBraveNewsDisplayAd.ts index c0c3be30be47..07ed1523aaed 100644 --- a/components/brave_new_tab_ui/stories/default/data/getBraveNewsDisplayAd.ts +++ b/components/brave_new_tab_ui/stories/default/data/getBraveNewsDisplayAd.ts @@ -7,7 +7,7 @@ let callCount = 0 export default function getBraveNewsDisplayAd (always: boolean = false) { callCount++ - const ad = (always || callCount % 2) + const ad: BraveToday.DisplayAd | null = (always || callCount % 2) ? { description: 'Technica', uuid: '0abc', @@ -19,5 +19,7 @@ export default function getBraveNewsDisplayAd (always: boolean = false) { dimensions: '1x3' } : null - return Promise.resolve(ad) + return new Promise(function (resolve) { + setTimeout(() => resolve(ad), 2400) + }) }