From 1e7e7a7b7aaf0cd679e7a085d77b412fd54a6dd0 Mon Sep 17 00:00:00 2001
From: infinite-persistence
 <64950861+infinite-persistence@users.noreply.github.com>
Date: Mon, 7 Feb 2022 12:59:20 -0800
Subject: [PATCH] Notifications: use fetched urls instead of resolving (#820)

* Fix avatar occasionally stuck in spaceman

## Issue
Sometimes, we'll see a channel profile (e.g. in upper-right, in channel selector) stuck in the fallback Spaceman image.

This is due to OptimizedImage always starting with a blank src, and updated later when the mounted size has been determined. ChannelThumbnail, which uses OptimizedImage, captured the `onError` due to blank src.

## Fix
Don't mount the <img> until the optimum size has been determined.

* FileThumbnail: skip resolve if thumbnail url is specified

* UriIndicator: skip resolve if channel info is specified

* Notifications: disable batch resolve + use fetched data if available + fallback to resolve if n/a

The fallback is using the individual resolve when no direct data is provided and claim is undefined.
---
 flow-typed/notification.js           |  3 ++
 ui/component/fileThumbnail/view.jsx  |  4 +-
 ui/component/notification/view.jsx   | 23 ++++++----
 ui/component/optimizedImage/view.jsx |  2 +-
 ui/component/uriIndicator/view.jsx   | 64 +++++++++++++++++++++-------
 ui/page/notifications/view.jsx       |  4 +-
 6 files changed, 70 insertions(+), 30 deletions(-)

diff --git a/flow-typed/notification.js b/flow-typed/notification.js
index 28bce14f5..dd863c575 100644
--- a/flow-typed/notification.js
+++ b/flow-typed/notification.js
@@ -115,11 +115,14 @@ declare type WebNotification = {
     },
     dynamic: {
       comment_author: string,
+      comment_author_thumbnail?: string,
       reply_author: string,
       hash: string,
       claim_title: string,
+      claim_thumbnail?: string,
       comment?: string,
       channel_url: string,
+      channel_thumbnail?: string,
     },
     email: {},
   },
diff --git a/ui/component/fileThumbnail/view.jsx b/ui/component/fileThumbnail/view.jsx
index a5ef83ab6..702dc9876 100644
--- a/ui/component/fileThumbnail/view.jsx
+++ b/ui/component/fileThumbnail/view.jsx
@@ -30,10 +30,10 @@ function FileThumbnail(props: Props) {
   const isGif = thumbnail && thumbnail.endsWith('gif');
 
   React.useEffect(() => {
-    if (!hasResolvedClaim && uri) {
+    if (!hasResolvedClaim && uri && !passedThumbnail) {
       doResolveUri(uri);
     }
-  }, [hasResolvedClaim, uri, doResolveUri]);
+  }, [hasResolvedClaim, uri, doResolveUri, passedThumbnail]);
 
   if (!allowGifs && isGif) {
     return (
diff --git a/ui/component/notification/view.jsx b/ui/component/notification/view.jsx
index 49ee78a41..8debe8700 100644
--- a/ui/component/notification/view.jsx
+++ b/ui/component/notification/view.jsx
@@ -50,11 +50,12 @@ export default function Notification(props: Props) {
   const stickerFromComment = isCommentNotification && commentText && parseSticker(commentText);
   const notificationTarget = getNotificationTarget();
 
-  const creatorIcon = (channelUrl) => (
-    <UriIndicator uri={channelUrl} link>
-      <ChannelThumbnail small uri={channelUrl} />
+  const creatorIcon = (channelUrl, channelThumbnail) => (
+    <UriIndicator uri={channelUrl} link channelInfo={{ uri: channelUrl, name: '' }}>
+      <ChannelThumbnail small thumbnailPreview={channelThumbnail} uri={channelThumbnail ? undefined : channelUrl} />
     </UriIndicator>
   );
+
   let channelUrl;
   let icon;
   switch (notification_rule) {
@@ -64,19 +65,19 @@ export default function Notification(props: Props) {
     case RULE.COMMENT:
     case RULE.CREATOR_COMMENT:
       channelUrl = notification_parameters.dynamic.comment_author;
-      icon = creatorIcon(channelUrl);
+      icon = creatorIcon(channelUrl, notification_parameters?.dynamic?.comment_author_thumbnail);
       break;
     case RULE.COMMENT_REPLY:
       channelUrl = notification_parameters.dynamic.reply_author;
-      icon = creatorIcon(channelUrl);
+      icon = creatorIcon(channelUrl, notification_parameters?.dynamic?.comment_author_thumbnail);
       break;
     case RULE.NEW_CONTENT:
       channelUrl = notification_parameters.dynamic.channel_url;
-      icon = creatorIcon(channelUrl);
+      icon = creatorIcon(channelUrl, notification_parameters?.dynamic?.channel_thumbnail);
       break;
     case RULE.NEW_LIVESTREAM:
       channelUrl = notification_parameters.dynamic.channel_url;
-      icon = creatorIcon(channelUrl);
+      icon = creatorIcon(channelUrl, notification_parameters?.dynamic?.channel_thumbnail);
       break;
     case RULE.DAILY_WATCH_AVAILABLE:
     case RULE.DAILY_WATCH_REMIND:
@@ -111,7 +112,7 @@ export default function Notification(props: Props) {
   let uriIndicator;
   const title = titleSplit.map((message, index) => {
     if (channelName === message) {
-      uriIndicator = <UriIndicator uri={channelUrl} link />;
+      uriIndicator = <UriIndicator uri={channelUrl} link channelInfo={{ uri: channelUrl, name: channelName }} />;
       fullTitle.push(' ');
       const resultTitle = fullTitle;
       fullTitle = [' '];
@@ -204,7 +205,11 @@ export default function Notification(props: Props) {
             </div>
 
             {notification_rule === RULE.NEW_CONTENT && (
-              <FileThumbnail uri={notification_parameters.device.target} className="notificationContent__thumbnail" />
+              <FileThumbnail
+                uri={notification_parameters.device.target}
+                thumbnail={notification_parameters?.dynamic?.claim_thumbnail}
+                className="notificationContent__thumbnail"
+              />
             )}
             {notification_rule === RULE.NEW_LIVESTREAM && (
               <FileThumbnail
diff --git a/ui/component/optimizedImage/view.jsx b/ui/component/optimizedImage/view.jsx
index 306ccae82..63b751b72 100644
--- a/ui/component/optimizedImage/view.jsx
+++ b/ui/component/optimizedImage/view.jsx
@@ -94,7 +94,7 @@ function OptimizedImage(props: Props) {
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [src]);
 
-  if (!src) {
+  if (!src || !optimizedSrc) {
     return null;
   }
 
diff --git a/ui/component/uriIndicator/view.jsx b/ui/component/uriIndicator/view.jsx
index 601fddfe3..e1f3aa1e4 100644
--- a/ui/component/uriIndicator/view.jsx
+++ b/ui/component/uriIndicator/view.jsx
@@ -4,9 +4,13 @@ import React from 'react';
 import classnames from 'classnames';
 import Button from 'component/button';
 
+type ChannelInfo = {
+  uri: string,
+  name: string,
+};
+
 type Props = {
   isResolvingUri: boolean,
-  channelUri: ?string,
   link: ?boolean,
   claim: ?Claim,
   hideAnonymous: boolean,
@@ -14,6 +18,7 @@ type Props = {
   // Possibly because the resolve function is an arrow function that is passed in props?
   resolveUri: (string) => void,
   uri: string,
+  channelInfo: ?ChannelInfo, // Direct channel info to use, bypassing the need to resolve 'uri'.
   // to allow for other elements to be nested within the UriIndicator
   children: ?Node,
   inline: boolean,
@@ -24,23 +29,53 @@ type Props = {
 
 class UriIndicator extends React.PureComponent<Props> {
   componentDidMount() {
-    this.resolve(this.props);
+    this.resolveClaim(this.props);
   }
 
   componentDidUpdate() {
-    this.resolve(this.props);
+    this.resolveClaim(this.props);
   }
 
-  resolve = (props: Props) => {
-    const { isResolvingUri, resolveUri, claim, uri } = props;
+  resolveClaim = (props: Props) => {
+    const { isResolvingUri, resolveUri, claim, uri, channelInfo } = props;
 
-    if (!isResolvingUri && claim === undefined && uri) {
+    if (!channelInfo && !isResolvingUri && claim === undefined && uri) {
       resolveUri(uri);
     }
   };
 
+  resolveState = (channelInfo: ?ChannelInfo, claim: ?Claim, isLinkType: ?boolean) => {
+    if (channelInfo) {
+      return {
+        hasChannelData: true,
+        isAnonymous: false,
+        channelName: channelInfo.name,
+        channelLink: isLinkType ? channelInfo.uri : false,
+      };
+    } else if (claim) {
+      const signingChannel = claim.signing_channel && claim.signing_channel.amount;
+      const isChannelClaim = claim.value_type === 'channel';
+      const channelClaim = isChannelClaim ? claim : claim.signing_channel;
+
+      return {
+        hasChannelData: Boolean(channelClaim),
+        isAnonymous: !signingChannel && !isChannelClaim,
+        channelName: channelClaim?.name,
+        channelLink: isLinkType ? channelClaim?.canonical_url || channelClaim?.permanent_url : false,
+      };
+    } else {
+      return {
+        hasChannelData: false,
+        isAnonymous: undefined,
+        channelName: undefined,
+        channelLink: undefined,
+      };
+    }
+  };
+
   render() {
     const {
+      channelInfo,
       link,
       isResolvingUri,
       claim,
@@ -52,7 +87,7 @@ class UriIndicator extends React.PureComponent<Props> {
       className,
     } = this.props;
 
-    if (!claim) {
+    if (!channelInfo && !claim) {
       return (
         <span className={classnames('empty', className)}>
           {isResolvingUri || claim === undefined ? __('Validating...') : __('[Removed]')}
@@ -60,9 +95,9 @@ class UriIndicator extends React.PureComponent<Props> {
       );
     }
 
-    const isChannelClaim = claim.value_type === 'channel';
-    const signingChannel = claim.signing_channel && claim.signing_channel.amount;
-    if (!signingChannel && !isChannelClaim) {
+    const data = this.resolveState(channelInfo, claim, link);
+
+    if (data.isAnonymous) {
       if (hideAnonymous) {
         return null;
       }
@@ -74,15 +109,12 @@ class UriIndicator extends React.PureComponent<Props> {
       );
     }
 
-    const channelClaim = isChannelClaim ? claim : claim.signing_channel;
-
-    if (channelClaim) {
-      const { name } = channelClaim;
-      const channelLink = link ? channelClaim.canonical_url || channelClaim.permanent_url : false;
+    if (data.hasChannelData) {
+      const { channelName, channelLink } = data;
 
       const inner = (
         <span dir="auto" className={classnames('channel-name', { 'channel-name--inline': inline })}>
-          {name}
+          {channelName}
         </span>
       );
 
diff --git a/ui/page/notifications/view.jsx b/ui/page/notifications/view.jsx
index ed587e766..88d405899 100644
--- a/ui/page/notifications/view.jsx
+++ b/ui/page/notifications/view.jsx
@@ -22,7 +22,7 @@ type Props = {
   unseenCount: number,
   doSeeAllNotifications: () => void,
   doReadNotifications: () => void,
-  doNotificationList: (?Array<string>) => void,
+  doNotificationList: (?Array<string>, ?boolean) => void,
   doNotificationCategories: () => void,
   activeChannel: ?ChannelClaim,
   doCommentReactList: (Array<string>) => Promise<any>,
@@ -97,7 +97,7 @@ export default function NotificationsPage(props: Props) {
         try {
           const matchingCategory = arrayNotificationCategories.find((category) => category.name === name);
           if (matchingCategory) {
-            doNotificationList(matchingCategory.types);
+            doNotificationList(matchingCategory.types, false);
           }
         } catch (e) {
           console.error(e); // eslint-disable-line no-console