Merge pull request #9743 from brave/news-displayads-fix-double-get

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<BraveToday.DisplayAd | undefined | null>(undefined)
   const [cardRef] = useScrollIntoView(props.shouldScrollIntoView || false)
   const onClick = useVisitDisplayAdClickHandler(props.onVisitDisplayAd, content ? { ad: content } : undefined)
   const innerRef = React.useRef<HTMLElement>(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<HTMLDivElement>(null)
-  const contentTriggerObserver = React.useRef<IntersectionObserver>()
-  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<Function>(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

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<Function>) {
+  // 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<HTMLDivElement>(null)
+  // Observer which will observe elements scroll position to trigger ad fetch
+  const contentTriggerObserver = React.useRef<IntersectionObserver>()
+  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]
+}

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<BraveToday.DisplayAd | null>(function (resolve) {
+    setTimeout(() => resolve(ad), 2400)
+  })
 }