From f13fc9c35abc48996e7c02c2c515c1098a457460 Mon Sep 17 00:00:00 2001
From: Jackson Harper <jacksonh@gmail.com>
Date: Wed, 21 Dec 2022 12:29:32 +0800
Subject: [PATCH 1/5] Only save labels from the modal when the user finishes
 selection

This prevents creating issues by calling the setlabels mutation
too frequently if a user is rapidly changing labels, if that
happens a failure in the API could cause the user's changes
to be overwritten.
---
 .../web/components/patterns/HighlightBar.tsx  | 77 ++++++++++++-------
 .../templates/article/ArticleActionsMenu.tsx  |  2 +-
 .../templates/article/HighlightsLayer.tsx     | 28 +++++++
 .../templates/article/HighlightsModal.tsx     |  2 +-
 .../templates/article/SetLabelsControl.tsx    | 10 ++-
 .../templates/article/SetLabelsModal.tsx      | 47 ++++++++---
 .../templates/homeFeed/HomeFeedContainer.tsx  |  2 +-
 .../mutations/setLabelsForHighlight.ts        |  8 +-
 .../networking/mutations/setLabelsMutation.ts |  3 +-
 .../web/pages/[username]/[slug]/index.tsx     |  5 +-
 10 files changed, 136 insertions(+), 48 deletions(-)

diff --git a/packages/web/components/patterns/HighlightBar.tsx b/packages/web/components/patterns/HighlightBar.tsx
index 92e5a6ae4..8fbde3083 100644
--- a/packages/web/components/patterns/HighlightBar.tsx
+++ b/packages/web/components/patterns/HighlightBar.tsx
@@ -6,9 +6,8 @@ import { AnchoredPopover } from '../patterns/AnchoredPopover'
 import { StyledText } from '../elements/StyledText'
 import { Button } from '../elements/Button'
 import { HStack, Box } from '../elements/LayoutPrimitives'
-import { TrashIcon } from '../elements/images/TrashIcon'
 import { PenWithColorIcon } from '../elements/images/PenWithColorIcon'
-import { Note, Trash, TrashSimple } from 'phosphor-react'
+import { Note, Tag, Trash } from 'phosphor-react'
 
 type PageCoordinates = {
   pageX: number
@@ -38,7 +37,7 @@ export function HighlightBar(props: HighlightBarProps): JSX.Element {
       <Box
         css={{
           width: '100%',
-          maxWidth: '240px',
+          maxWidth: '300px',
           height: '48px',
           position: 'fixed',
           background: '$grayBg',
@@ -62,7 +61,7 @@ export function HighlightBar(props: HighlightBarProps): JSX.Element {
       <Box
         css={{
           width: '100%',
-          maxWidth: '240px',
+          maxWidth: '300px',
           height: '48px',
           position: 'absolute',
           background: '$grayBg',
@@ -127,28 +126,54 @@ function BarContent(props: HighlightBarProps): JSX.Element {
           </HStack>
         </Button>
       ) : (
-        <Button
-          style="plainIcon"
-          title="Remove Highlight"
-          onClick={() => props.handleButtonClick('delete')}
-          css={{ color: '$readerFont', height: '100%', m: 0, p: 0 }}
-        >
-          <HStack css={{ height: '100%', alignItems: 'center' }}>
-            <Trash size={24} color={theme.colors.omnivoreRed.toString()} />
-            <StyledText
-              style="body"
-              css={{
-                pl: '12px',
-                m: '0px',
-                color: '$readerFont',
-                fontWeight: '400',
-                fontSize: '16px',
-              }}
-            >
-              Delete
-            </StyledText>
-          </HStack>
-        </Button>
+        <>
+          <Button
+            style="plainIcon"
+            title="Remove Highlight"
+            onClick={() => props.handleButtonClick('delete')}
+            css={{ color: '$readerFont', height: '100%', m: 0, p: 0 }}
+          >
+            <HStack css={{ height: '100%', alignItems: 'center' }}>
+              <Trash size={24} color={theme.colors.omnivoreRed.toString()} />
+              <StyledText
+                style="body"
+                css={{
+                  pl: '12px',
+                  m: '0px',
+                  color: '$readerFont',
+                  fontWeight: '400',
+                  fontSize: '16px',
+                }}
+              >
+                Delete
+              </StyledText>
+            </HStack>
+          </Button>
+          <Separator />
+
+          <Button
+            style="plainIcon"
+            title="Set Labels"
+            onClick={() => props.handleButtonClick('setHighlightLabels')}
+            css={{ color: '$readerFont', height: '100%', m: 0, p: 0 }}
+          >
+            <HStack css={{ height: '100%', alignItems: 'center' }}>
+              <Tag size={24} color={theme.colors.readerFont.toString()} />
+              <StyledText
+                style="body"
+                css={{
+                  pl: '12px',
+                  m: '0px',
+                  color: '$readerFont',
+                  fontWeight: '400',
+                  fontSize: '16px',
+                }}
+              >
+                Labels
+              </StyledText>
+            </HStack>
+          </Button>
+        </>
       )}
       <Separator />
       <Button
diff --git a/packages/web/components/templates/article/ArticleActionsMenu.tsx b/packages/web/components/templates/article/ArticleActionsMenu.tsx
index 2469f5798..195f176ff 100644
--- a/packages/web/components/templates/article/ArticleActionsMenu.tsx
+++ b/packages/web/components/templates/article/ArticleActionsMenu.tsx
@@ -147,7 +147,7 @@ export function ArticleActionsMenu(
                   }
                   return Promise.resolve(undefined)
                 }}
-                onSave={(labels) => {
+                onLabelsChanged={(labels) => {
                   props.articleActionHandler('refreshLabels', labels)
                 }}
               />
diff --git a/packages/web/components/templates/article/HighlightsLayer.tsx b/packages/web/components/templates/article/HighlightsLayer.tsx
index c2663bac9..8b964782c 100644
--- a/packages/web/components/templates/article/HighlightsLayer.tsx
+++ b/packages/web/components/templates/article/HighlightsLayer.tsx
@@ -24,6 +24,9 @@ import { useCanShareNative } from '../../../lib/hooks/useCanShareNative'
 import { showErrorToast } from '../../../lib/toastHelpers'
 import { ArticleMutations } from '../../../lib/articleActions'
 import { isTouchScreenDevice } from '../../../lib/deviceType'
+import { SetLabelsModal } from './SetLabelsModal'
+import { setLabelsForHighlight } from '../../../lib/networking/mutations/setLabelsForHighlight'
+import { Label } from '../../../lib/networking/fragments/labelFragment'
 
 type HighlightsLayerProps = {
   highlights: Highlight[]
@@ -75,6 +78,10 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
     false //noteModal.open,
   )
 
+  const [labelsTarget, setLabelsTarget] = useState<Highlight | undefined>(
+    undefined
+  )
+
   const canShareNative = useCanShareNative()
 
   // Load the highlights
@@ -399,6 +406,8 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
               actionID: 'setHighlightLabels',
               highlightID: focusedHighlight?.id,
             })
+          } else {
+            setLabelsTarget(focusedHighlight)
           }
           break
       }
@@ -568,6 +577,25 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
     )
   }
 
+  if (labelsTarget) {
+    return (
+      <SetLabelsModal
+        provider={labelsTarget}
+        onOpenChange={function (open: boolean): void {
+          setLabelsTarget(undefined)
+        }}
+        onLabelsChanged={function (labels: Label[] | undefined): void {}}
+        save={function (labels: Label[]): Promise<Label[] | undefined> {
+          const result = setLabelsForHighlight(
+            labelsTarget.id,
+            labels.map((label) => label.id)
+          )
+          return result
+        }}
+      />
+    )
+  }
+
   if (
     highlightModalAction?.highlightModalAction == 'share' &&
     highlightModalAction.highlight
diff --git a/packages/web/components/templates/article/HighlightsModal.tsx b/packages/web/components/templates/article/HighlightsModal.tsx
index c59e080da..7bce504bf 100644
--- a/packages/web/components/templates/article/HighlightsModal.tsx
+++ b/packages/web/components/templates/article/HighlightsModal.tsx
@@ -105,7 +105,7 @@ export function HighlightsModal(props: HighlightsModalProps): JSX.Element {
           onOpenChange={function (open: boolean): void {
             setLabelsTarget(undefined)
           }}
-          onSave={function (labels: Label[] | undefined): void {
+          onLabelsChanged={function (labels: Label[]): void {
             updateState({})
           }}
           save={function (labels: Label[]): Promise<Label[] | undefined> {
diff --git a/packages/web/components/templates/article/SetLabelsControl.tsx b/packages/web/components/templates/article/SetLabelsControl.tsx
index 3d4254506..04deb2b09 100644
--- a/packages/web/components/templates/article/SetLabelsControl.tsx
+++ b/packages/web/components/templates/article/SetLabelsControl.tsx
@@ -20,7 +20,7 @@ export interface LabelsProvider {
 
 type SetLabelsControlProps = {
   provider: LabelsProvider
-  onSave: (labels: Label[] | undefined) => void
+  onLabelsChanged: (labels: Label[]) => void
   save: (labels: Label[]) => Promise<Label[] | undefined>
 }
 
@@ -269,9 +269,11 @@ export function SetLabelsControl(props: SetLabelsControlProps): JSX.Element {
       setSelectedLabels(newSelectedLabels)
 
       const result = await props.save(newSelectedLabels)
-      props.provider.labels = result
-      if (props.onSave) {
-        props.onSave(result)
+      if (result) {
+        props.provider.labels = result
+        if (props.onLabelsChanged) {
+          props.onLabelsChanged(result)
+        }
       }
 
       revalidate()
diff --git a/packages/web/components/templates/article/SetLabelsModal.tsx b/packages/web/components/templates/article/SetLabelsModal.tsx
index 022136bf3..ed7edac6b 100644
--- a/packages/web/components/templates/article/SetLabelsModal.tsx
+++ b/packages/web/components/templates/article/SetLabelsModal.tsx
@@ -1,41 +1,66 @@
+import { useCallback, useState } from 'react'
 import { Label } from '../../../lib/networking/fragments/labelFragment'
-import { ArticleAttributes } from '../../../lib/networking/queries/useGetArticleQuery'
-import { Button } from '../../elements/Button'
-import { CrossIcon } from '../../elements/images/CrossIcon'
-import { HStack, SpanBox, VStack } from '../../elements/LayoutPrimitives'
+import { showErrorToast, showSuccessToast } from '../../../lib/toastHelpers'
+import { SpanBox, VStack } from '../../elements/LayoutPrimitives'
 import {
   ModalRoot,
   ModalOverlay,
   ModalContent,
   ModalTitleBar,
 } from '../../elements/ModalPrimitives'
-import { StyledText } from '../../elements/StyledText'
-import { theme } from '../../tokens/stitches.config'
 import { LabelsProvider, SetLabelsControl } from './SetLabelsControl'
 
 type SetLabelsModalProps = {
   provider: LabelsProvider
   onOpenChange: (open: boolean) => void
-  onSave: (labels: Label[] | undefined) => void
+  onLabelsChanged: (labels: Label[]) => void
   save: (labels: Label[]) => Promise<Label[] | undefined>
 }
 
 export function SetLabelsModal(props: SetLabelsModalProps): JSX.Element {
+  const [selectedLabels, setSelectedLabels] = useState(props.provider.labels)
+  const onOpenChange = useCallback(
+    async (open: boolean) => {
+      if (selectedLabels) {
+        const result = await props.save(selectedLabels)
+        if (result) {
+          props.onLabelsChanged(result)
+        } else {
+          showErrorToast('Error updating labels')
+        }
+      }
+      props.onOpenChange(open)
+    },
+    [props.onOpenChange]
+  )
+
+  const handleSave = useCallback(
+    (labels: Label[]) => {
+      setSelectedLabels(labels)
+      return Promise.resolve(labels)
+    },
+    [selectedLabels, setSelectedLabels]
+  )
+
   return (
-    <ModalRoot defaultOpen onOpenChange={props.onOpenChange}>
+    <ModalRoot defaultOpen onOpenChange={onOpenChange}>
       <ModalOverlay />
       <ModalContent
         css={{ border: '1px solid $grayBorder' }}
         onPointerDownOutside={(event) => {
           event.preventDefault()
-          props.onOpenChange(false)
+          onOpenChange(false)
         }}
       >
         <VStack distribution="start" css={{ height: '100%' }}>
           <SpanBox css={{ p: '16px', width: '100%' }}>
-            <ModalTitleBar title="Labels" onOpenChange={props.onOpenChange} />
+            <ModalTitleBar title="Labels" onOpenChange={onOpenChange} />
           </SpanBox>
-          <SetLabelsControl {...props} />
+          <SetLabelsControl
+            provider={props.provider}
+            save={handleSave}
+            onLabelsChanged={props.onLabelsChanged}
+          />
         </VStack>
       </ModalContent>
     </ModalRoot>
diff --git a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx
index 3ddf1f653..bd80bb54d 100644
--- a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx
+++ b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx
@@ -1205,7 +1205,7 @@ function HomeFeedGrid(props: HomeFeedContentProps): JSX.Element {
       {props.labelsTarget?.node.id && (
         <SetLabelsModal
           provider={props.labelsTarget.node}
-          onSave={(labels: Label[] | undefined) => {
+          onLabelsChanged={(labels: Label[]) => {
             if (props.labelsTarget) {
               props.labelsTarget.node.labels = labels
               updateState({})
diff --git a/packages/web/lib/networking/mutations/setLabelsForHighlight.ts b/packages/web/lib/networking/mutations/setLabelsForHighlight.ts
index e926314d3..c9547670f 100644
--- a/packages/web/lib/networking/mutations/setLabelsForHighlight.ts
+++ b/packages/web/lib/networking/mutations/setLabelsForHighlight.ts
@@ -4,11 +4,11 @@ import { gqlFetcher } from '../networkHelpers'
 
 export type SetLabelsForHighlightResult = {
   setLabelsForHighlight: SetLabelsForHighlight
-  errorCodes?: unknown[]
 }
 
 type SetLabelsForHighlight = {
   labels: Label[]
+  errorCodes?: unknown[]
 }
 
 export async function setLabelsForHighlight(
@@ -35,7 +35,11 @@ export async function setLabelsForHighlight(
     const data = (await gqlFetcher(mutation, {
       input: { highlightId, labelIds },
     })) as SetLabelsForHighlightResult
-    return data.errorCodes ? undefined : data.setLabelsForHighlight.labels
+    console.log(' -- errorCodes', data.setLabelsForHighlight.errorCodes)
+
+    return data.setLabelsForHighlight.errorCodes
+      ? undefined
+      : data.setLabelsForHighlight.labels
   } catch (error) {
     console.log('setLabelsForHighlightInput error', error)
     return undefined
diff --git a/packages/web/lib/networking/mutations/setLabelsMutation.ts b/packages/web/lib/networking/mutations/setLabelsMutation.ts
index cf0bb654b..dabdb4ec2 100644
--- a/packages/web/lib/networking/mutations/setLabelsMutation.ts
+++ b/packages/web/lib/networking/mutations/setLabelsMutation.ts
@@ -35,9 +35,10 @@ export async function setLabelsMutation(
     const data = (await gqlFetcher(mutation, {
       input: { pageId, labelIds },
     })) as SetLabelsResult
+    console.log(' -- errorCodes', data.errorCodes)
     return data.errorCodes ? undefined : data.setLabels.labels
   } catch (error) {
-    console.log('SetLabelsOutput error', error)
+    console.log(' -- SetLabelsOutput error', error)
     return undefined
   }
 }
diff --git a/packages/web/pages/[username]/[slug]/index.tsx b/packages/web/pages/[username]/[slug]/index.tsx
index 2bbf26d0f..29c9565a0 100644
--- a/packages/web/pages/[username]/[slug]/index.tsx
+++ b/packages/web/pages/[username]/[slug]/index.tsx
@@ -126,6 +126,7 @@ export default function Home(): JSX.Element {
           }
           break
         case 'refreshLabels':
+          console.log('refreshing labels: ', arg)
           setLabels(arg as Label[])
           break
         case 'showHighlights':
@@ -264,6 +265,8 @@ export default function Home(): JSX.Element {
     return <LoadingView />
   }
 
+  console.log('reader settings modal: ', readerSettings)
+
   return (
     <PrimaryLayout
       pageTestId="home-page-tag"
@@ -367,7 +370,7 @@ export default function Home(): JSX.Element {
       {article && readerSettings.showSetLabelsModal && (
         <SetLabelsModal
           provider={article}
-          onSave={(labels: Label[] | undefined) => {
+          onLabelsChanged={(labels: Label[]) => {
             actionHandler('refreshLabels', labels)
           }}
           save={(labels: Label[]) => {

From 1fddbc3b5843acab5b02a4884dbfccf789a01295 Mon Sep 17 00:00:00 2001
From: Jackson Harper <jacksonh@gmail.com>
Date: Wed, 21 Dec 2022 12:54:50 +0800
Subject: [PATCH 2/5] Remove debug code

---
 packages/web/pages/[username]/[slug]/index.tsx | 2 --
 1 file changed, 2 deletions(-)

diff --git a/packages/web/pages/[username]/[slug]/index.tsx b/packages/web/pages/[username]/[slug]/index.tsx
index 29c9565a0..8cf4f0226 100644
--- a/packages/web/pages/[username]/[slug]/index.tsx
+++ b/packages/web/pages/[username]/[slug]/index.tsx
@@ -265,8 +265,6 @@ export default function Home(): JSX.Element {
     return <LoadingView />
   }
 
-  console.log('reader settings modal: ', readerSettings)
-
   return (
     <PrimaryLayout
       pageTestId="home-page-tag"

From a15af3f51400000bd90313a6d176c1114e98208b Mon Sep 17 00:00:00 2001
From: Jackson Harper <jacksonh@gmail.com>
Date: Wed, 21 Dec 2022 12:55:08 +0800
Subject: [PATCH 3/5] Different sizing for highlight bar if displaying the
 extra image

---
 packages/web/components/patterns/HighlightBar.tsx | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/web/components/patterns/HighlightBar.tsx b/packages/web/components/patterns/HighlightBar.tsx
index 8fbde3083..d06aba400 100644
--- a/packages/web/components/patterns/HighlightBar.tsx
+++ b/packages/web/components/patterns/HighlightBar.tsx
@@ -37,7 +37,7 @@ export function HighlightBar(props: HighlightBarProps): JSX.Element {
       <Box
         css={{
           width: '100%',
-          maxWidth: '300px',
+          maxWidth: props.isNewHighlight ? '280px' : '330px',
           height: '48px',
           position: 'fixed',
           background: '$grayBg',
@@ -46,7 +46,7 @@ export function HighlightBar(props: HighlightBarProps): JSX.Element {
           boxShadow: theme.shadows.cardBoxShadow.toString(),
           bottom: 'calc(38px + env(safe-area-inset-bottom, 40px))',
           '@smDown': {
-            maxWidth: '80%',
+            maxWidth: '85%',
             bottom: `calc(28px + ${
               isAndroid() ? 30 : 0
             }px + env(safe-area-inset-bottom, 40px))`,
@@ -61,7 +61,7 @@ export function HighlightBar(props: HighlightBarProps): JSX.Element {
       <Box
         css={{
           width: '100%',
-          maxWidth: '300px',
+          maxWidth: props.isNewHighlight ? '280px' : '330px',
           height: '48px',
           position: 'absolute',
           background: '$grayBg',

From f8b828a0700a9cddf61b09b3414dc70f8ff56231 Mon Sep 17 00:00:00 2001
From: Jackson Harper <jacksonh@gmail.com>
Date: Wed, 21 Dec 2022 12:59:12 +0800
Subject: [PATCH 4/5] Disable empty arrow, this func isnt needed as we call
 save

---
 packages/web/components/templates/article/HighlightsLayer.tsx | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/packages/web/components/templates/article/HighlightsLayer.tsx b/packages/web/components/templates/article/HighlightsLayer.tsx
index 8b964782c..ec70d9f63 100644
--- a/packages/web/components/templates/article/HighlightsLayer.tsx
+++ b/packages/web/components/templates/article/HighlightsLayer.tsx
@@ -584,7 +584,8 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
         onOpenChange={function (open: boolean): void {
           setLabelsTarget(undefined)
         }}
-        onLabelsChanged={function (labels: Label[] | undefined): void {}}
+        // eslint-disable-next-line @typescript-eslint/no-empty-function
+        onLabelsChanged={function (labels: Label[]): void {}}
         save={function (labels: Label[]): Promise<Label[] | undefined> {
           const result = setLabelsForHighlight(
             labelsTarget.id,

From 79ae04215eeb73ab702b8ed5d3bd695a5770f3f9 Mon Sep 17 00:00:00 2001
From: Jackson Harper <jacksonh@gmail.com>
Date: Wed, 21 Dec 2022 13:14:49 +0800
Subject: [PATCH 5/5] Remove unused imports

---
 .../templates/article/ArticleContainer.tsx    | 27 +++----------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/packages/web/components/templates/article/ArticleContainer.tsx b/packages/web/components/templates/article/ArticleContainer.tsx
index b13845f77..33d488b72 100644
--- a/packages/web/components/templates/article/ArticleContainer.tsx
+++ b/packages/web/components/templates/article/ArticleContainer.tsx
@@ -1,40 +1,21 @@
 import { ArticleAttributes } from '../../../lib/networking/queries/useGetArticleQuery'
 import { Article } from './../../../components/templates/article/Article'
-import {
-  Blockquote,
-  Box,
-  HStack,
-  SpanBox,
-  VStack,
-} from './../../elements/LayoutPrimitives'
-import { StyledText, StyledTextSpan } from './../../elements/StyledText'
+import { Box, HStack, SpanBox, VStack } from './../../elements/LayoutPrimitives'
+import { StyledText } from './../../elements/StyledText'
 import { ArticleSubtitle } from './../../patterns/ArticleSubtitle'
-import { styled, theme, ThemeId } from './../../tokens/stitches.config'
+import { theme, ThemeId } from './../../tokens/stitches.config'
 import { HighlightsLayer } from '../../templates/article/HighlightsLayer'
 import { Button } from '../../elements/Button'
-import {
-  MutableRefObject,
-  useEffect,
-  useState,
-  useRef,
-  useReducer,
-  useMemo,
-} from 'react'
+import { useEffect, useState, useRef, useMemo } from 'react'
 import { ReportIssuesModal } from './ReportIssuesModal'
 import { reportIssueMutation } from '../../../lib/networking/mutations/reportIssueMutation'
-import { ArticleHeaderToolbar } from './ArticleHeaderToolbar'
 import { userPersonalizationMutation } from '../../../lib/networking/mutations/userPersonalizationMutation'
 import { updateTheme, updateThemeLocally } from '../../../lib/themeUpdater'
 import { ArticleMutations } from '../../../lib/articleActions'
 import { LabelChip } from '../../elements/LabelChip'
 import { Label } from '../../../lib/networking/fragments/labelFragment'
-import {
-  HighlightLocation,
-  makeHighlightStartEndOffset,
-} from '../../../lib/highlights/highlightGenerator'
 import { Recommendation } from '../../../lib/networking/queries/useGetLibraryItemsQuery'
 import { Avatar } from '../../elements/Avatar'
-import { Sparkle } from 'phosphor-react'
 
 type ArticleContainerProps = {
   article: ArticleAttributes