Merge pull request #1565 from omnivore-app/fix/set-labels-conflicts

Only save labels from the modal when the user finishes selection

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: props.isNewHighlight ? '280px' : '330px',
           height: '48px',
           position: 'fixed',
           background: '$grayBg',
@@ -47,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))`,
@@ -62,7 +61,7 @@ export function HighlightBar(props: HighlightBarProps): JSX.Element {
       <Box
         css={{
           width: '100%',
-          maxWidth: '240px',
+          maxWidth: props.isNewHighlight ? '280px' : '330px',
           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

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)
                 }}
               />

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

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,26 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
     )
   }
 
+  if (labelsTarget) {
+    return (
+      <SetLabelsModal
+        provider={labelsTarget}
+        onOpenChange={function (open: boolean): void {
+          setLabelsTarget(undefined)
+        }}
+        // 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,
+            labels.map((label) => label.id)
+          )
+          return result
+        }}
+      />
+    )
+  }
+
   if (
     highlightModalAction?.highlightModalAction == 'share' &&
     highlightModalAction.highlight

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> {

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()

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>

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({})

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

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
   }
 }

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':
@@ -367,7 +368,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[]) => {