PORTALS-2768: code review improvements

jay-hodgson · Feb 1, 2024 · 596312c · 596312c
1 parent 6d85a93
commit 596312c
Showing 1 changed file with 44 additions and 50 deletions.
diff --git a/packages/synapse-react-client/src/components/HasAccess/HasAccessV2.tsx b/packages/synapse-react-client/src/components/HasAccess/HasAccessV2.tsx
@@ -100,13 +100,6 @@ function AccessIcon(props: { downloadType: FileHandleDownloadTypeEnum }) {
   return <></>
 }
 
-function isFileHandleDownloadTypeUndefined(
-  fileHandleDownloadType: FileHandleDownloadTypeEnum | undefined,
-) {
-  // note, this can't be "if (!downloadType)" since DownloadTypeEnum has a 0 value (which is falsy)
-  return fileHandleDownloadType === undefined
-}
-
 /**
  * Determines whether an Entity is accessible for download, or if it is blocked by the ACL or unmet Access Requirements.
  *
@@ -242,14 +235,46 @@ export function HasAccessV2(props: HasAccessProps) {
     )
   }, [accessToken, entityId])
 
-  // Show Access Requirements
-  const accessRequirementsJsx = useMemo(() => {
-    if (
-      !restrictionInformation ||
-      isFileHandleDownloadTypeUndefined(fileHandleDownloadType)
-    ) {
+  // Sign-in wrapped icon or icon alone
+  const iconContainer = useMemo(() => {
+    return fileHandleDownloadType ===
+      FileHandleDownloadTypeEnum.AccessBlockedToAnonymous ? (
+      <Button
+        sx={buttonSx}
+        className={SRC_SIGN_IN_CLASS}
+        onClick={ev => {
+          if (ev.isTrusted) {
+            /*
+                There is a tricky problem - 
+                The portals listens to click events for buttons with the class SRC_SIGN_IN_CLASS set, it listens to this event
+                so that it can display the login modal.
+
+                This button has an svg inside of it which is problematic because more often than not clicking this button will 
+                instead click that svg. The event listener in the portals will break as a result.
+
+                Though the svg may get the actual click event, because of event bubbling this button will get its onClick called.
+                Once onClick is called we can manually dispatch an event off of this button. This does pose a problem, we end up in a 
+                infinite loop because this button keeps disptaching click events, so we can use the isTrusted to recognize if onClick was
+                triggered programmatically or by user click. Lastly, using { bubbles: true } ensures the event bubbles up to the document level.
+
+              */
+            const clickEvent = new MouseEvent('click', { bubbles: true })
+            ev.currentTarget.dispatchEvent(clickEvent)
+          }
+        }}
+      >
+        <AccessIcon downloadType={fileHandleDownloadType} />
+      </Button>
+    ) : (
+      <AccessIcon downloadType={fileHandleDownloadType!} />
+    )
+  }, [fileHandleDownloadType])
+
+  // Access Requirements icon or Icon Container
+  const accessRequirementsJsxOrIconContainer = useMemo(() => {
+    if (!restrictionInformation || !fileHandleDownloadType) {
       // loading
-      return null
+      return <></>
     }
     const hasUnmetAccessRequirement =
       restrictionInformation?.hasUnmetAccessRequirement
@@ -260,7 +285,7 @@ export function HasAccessV2(props: HasAccessProps) {
       linkText = 'Request Access'
     } else if (RestrictionLevel.OPEN === restrictionLevel) {
       // they need to sign in
-      return null
+      return iconContainer
     } else {
       linkText = 'View Terms'
     }
@@ -271,7 +296,7 @@ export function HasAccessV2(props: HasAccessProps) {
           className={props.className}
           onClick={handleGetAccess}
         >
-          <AccessIcon downloadType={fileHandleDownloadType!} />
+          <AccessIcon downloadType={fileHandleDownloadType} />
           {showButtonText && <Box ml="5px">{linkText}</Box>}
         </Button>
         {displayAccessRequirement && (
@@ -295,48 +320,17 @@ export function HasAccessV2(props: HasAccessProps) {
     props.className,
     fileHandleDownloadType,
     showButtonText,
+    iconContainer,
   ])
 
-  if (isFileHandleDownloadTypeUndefined(fileHandleDownloadType)) {
+  if (!fileHandleDownloadType) {
     // loading
     return <></>
   }
-  const iconContainer =
-    fileHandleDownloadType ===
-    FileHandleDownloadTypeEnum.AccessBlockedToAnonymous ? (
-      <Button
-        sx={buttonSx}
-        className={SRC_SIGN_IN_CLASS}
-        onClick={ev => {
-          if (ev.isTrusted) {
-            /*
-                There is a tricky problem - 
-                The portals listens to click events for buttons with the class SRC_SIGN_IN_CLASS set, it listens to this event
-                so that it can display the login modal.
-
-                This button has an svg inside of it which is problematic because more often than not clicking this button will 
-                instead click that svg. The event listener in the portals will break as a result.
-
-                Though the svg may get the actual click event, because of event bubbling this button will get its onClick called.
-                Once onClick is called we can manually dispatch an event off of this button. This does pose a problem, we end up in a 
-                infinite loop because this button keeps disptaching click events, so we can use the isTrusted to recognize if onClick was
-                triggered programmatically or by user click. Lastly, using { bubbles: true } ensures the event bubbles up to the document level.
-
-              */
-            const clickEvent = new MouseEvent('click', { bubbles: true })
-            ev.currentTarget.dispatchEvent(clickEvent)
-          }
-        }}
-      >
-        <AccessIcon downloadType={fileHandleDownloadType} />
-      </Button>
-    ) : (
-      <AccessIcon downloadType={fileHandleDownloadType!} />
-    )
 
   return (
     <span style={{ whiteSpace: 'nowrap' }}>
-      {accessRequirementsJsx ? accessRequirementsJsx : iconContainer}
+      {accessRequirementsJsxOrIconContainer}
     </span>
   )
 }