make attachments editable in the UI (keybase#24180)

* make attachments editable in the UI

* consolidate, review feedback

* fix test

* fix other test

shared/actions/chat2/index.tsx

@@ -1414,13 +1414,15 @@ function* messageEdit(
     return
   }
 
-  if (message.type === 'text') {
+  if (message.type === 'text' || message.type === 'attachment') {
     // Skip if the content is the same
-    if (message.text.stringValue() === text.stringValue()) {
+    if (message.type === 'text' && message.text.stringValue() === text.stringValue()) {
+      yield Saga.put(Chat2Gen.createMessageSetEditing({conversationIDKey, ordinal: null}))
+      return
+    } else if (message.type === 'attachment' && message.title === text.stringValue()) {
       yield Saga.put(Chat2Gen.createMessageSetEditing({conversationIDKey, ordinal: null}))
       return
     }
-
     const meta = Constants.getMeta(state, conversationIDKey)
     const tlfName = meta.tlfname
     const clientPrev = Constants.getClientPrev(state, conversationIDKey)

shared/chat/conversation/messages/attachment/container.tsx

@@ -7,17 +7,24 @@ import AudioAttachment from './audio'
 type Props = {
   message: Types.MessageAttachment
   toggleMessageMenu: () => void
+  isHighlighted?: boolean
 }
 
 const Attachment = React.memo((props: Props) => {
-  const {message, toggleMessageMenu} = props
+  const {isHighlighted, message, toggleMessageMenu} = props
   switch (message.attachmentType) {
     case 'image':
-      return <ImageAttachment message={message} toggleMessageMenu={toggleMessageMenu} />
+      return (
+        <ImageAttachment
+          message={message}
+          toggleMessageMenu={toggleMessageMenu}
+          isHighlighted={isHighlighted}
+        />
+      )
     case 'audio':
       return <AudioAttachment message={message} />
     default:
-      return <FileAttachment message={message} />
+      return <FileAttachment message={message} isHighlighted={isHighlighted} />
   }
 })
 

shared/chat/conversation/messages/attachment/file/container.tsx

@@ -1,4 +1,5 @@
 import * as Types from '../../../../../constants/types/chat2'
+import * as Constants from '../../../../../constants/chat2'
 import * as CryptoTypes from '../../../../../constants/types/crypto'
 import * as Tabs from '../../../../../constants/tabs'
 import * as FsGen from '../../../../../actions/fs-gen'
@@ -11,11 +12,16 @@ import {isPathSaltpack} from '../../../../../constants/crypto'
 import File from '.'
 
 type OwnProps = {
+  isHighlighted?: boolean
   message: Types.MessageAttachment
 }
 
 export default Container.connect(
-  () => ({}),
+  (state: Container.TypedState, ownProps: OwnProps) => {
+    const editInfo = Constants.getEditInfo(state, ownProps.message.conversationIDKey)
+    const isEditing = !!(editInfo && editInfo.ordinal === ownProps.message.ordinal)
+    return {isEditing}
+  },
   dispatch => ({
     _onDownload: (message: Types.MessageAttachment) => {
       switch (message.transferState) {
@@ -47,7 +53,7 @@ export default Container.connect(
         dispatch(FsGen.createOpenLocalPathInSystemFileManager({localPath: message.downloadPath}))
     },
   }),
-  (_, dispatchProps, ownProps: OwnProps) => {
+  (stateProps, dispatchProps, ownProps: OwnProps) => {
     const message = ownProps.message
     const {downloadPath, transferState} = message
     const arrowColor = Container.isMobile
@@ -64,6 +70,8 @@ export default Container.connect(
       errorMsg: message.transferErrMsg || '',
       fileName: message.fileName,
       hasProgress,
+      isEditing: stateProps.isEditing,
+      isHighlighted: ownProps.isHighlighted,
       isSaltpackFile: isPathSaltpack(message.fileName),
       message,
       onDownload: () => {

shared/chat/conversation/messages/attachment/file/index.tsx

@@ -5,7 +5,7 @@ import * as Types from '../../../../../constants/types/chat2'
 import * as CryptoTypes from '../../../../../constants/types/crypto'
 import * as Constants from '../../../../../constants/chat2'
 import * as Styles from '../../../../../styles'
-import {ShowToastAfterSaving} from '../shared'
+import {getEditStyle, ShowToastAfterSaving} from '../shared'
 import {useMemo} from '../../../../../util/memoize'
 import {isPathSaltpackEncrypted, isPathSaltpackSigned, Operations} from '../../../../../constants/crypto'
 
@@ -20,13 +20,15 @@ type Props = {
   transferState: Types.MessageAttachmentTransferState
   hasProgress: boolean
   errorMsg: string
+  isHighlighted?: boolean
+  isEditing: boolean
   isSaltpackFile: boolean
   onSaltpackFileOpen: (path: string, operation: CryptoTypes.Operations) => void
 }
 
 const FileAttachment = React.memo((props: Props) => {
   const progressLabel = Constants.messageAttachmentTransferStateToProgressLabel(props.transferState)
-  const {message, isSaltpackFile} = props
+  const {message, isSaltpackFile, isEditing, isHighlighted} = props
   const iconType = isSaltpackFile ? 'icon-file-saltpack-32' : 'icon-file-32'
   const operation = isPathSaltpackEncrypted(props.fileName)
     ? Operations.Decrypt
@@ -38,7 +40,7 @@ const FileAttachment = React.memo((props: Props) => {
   return (
     <>
       <ShowToastAfterSaving transferState={props.transferState} />
-      <Kb.Box style={styles.containerStyle}>
+      <Kb.Box style={Styles.collapseStyles([styles.containerStyle, getEditStyle(isEditing, isHighlighted)])}>
         <Kb.Box2 direction="horizontal" fullWidth={true} gap="tiny" centerChildren={true}>
           <Kb.Icon type={iconType} style={styles.iconStyle} onClick={props.onDownload} />
           <Kb.Box2 direction="vertical" fullWidth={true} style={styles.titleStyle}>
@@ -47,20 +49,34 @@ const FileAttachment = React.memo((props: Props) => {
               <Kb.Text
                 type="BodySemibold"
                 onClick={props.onDownload}
-                style={Styles.collapseStyles([isSaltpackFile && styles.saltpackFileName])}
+                style={Styles.collapseStyles([
+                  isSaltpackFile && styles.saltpackFileName,
+                  getEditStyle(isEditing, isHighlighted),
+                ])}
               >
                 {props.fileName}
               </Kb.Text>
             ) : (
-              <Kb.Markdown meta={wrappedMeta} selectable={true}>
+              <Kb.Markdown
+                meta={wrappedMeta}
+                selectable={true}
+                style={getEditStyle(isEditing, isHighlighted)}
+                styleOverride={
+                  Styles.isMobile ? {paragraph: getEditStyle(isEditing, isHighlighted)} : undefined
+                }
+                allowFontScaling={true}
+              >
                 {props.title}
               </Kb.Markdown>
             )}
             {props.fileName !== props.title && (
               <Kb.Text
                 type="BodyTiny"
                 onClick={props.onDownload}
-                style={Styles.collapseStyles([isSaltpackFile && styles.saltpackFileName])}
+                style={Styles.collapseStyles([
+                  isSaltpackFile && styles.saltpackFileName,
+                  getEditStyle(isEditing, isHighlighted),
+                ])}
               >
                 {props.fileName}
               </Kb.Text>

shared/chat/conversation/messages/attachment/image/container.tsx

@@ -10,6 +10,7 @@ import {imgMaxWidth} from './image-render'
 type OwnProps = {
   message: Types.MessageAttachment
   toggleMessageMenu: () => void
+  isHighlighted?: boolean
 }
 
 const mapDispatchToProps = (dispatch: Container.TypedDispatch) => ({
@@ -46,9 +47,13 @@ const mapDispatchToProps = (dispatch: Container.TypedDispatch) => ({
 })
 
 export default Container.connect(
-  () => ({}),
+  (state: Container.TypedState, ownProps: OwnProps) => {
+    const editInfo = Constants.getEditInfo(state, ownProps.message.conversationIDKey)
+    const isEditing = !!(editInfo && editInfo.ordinal === ownProps.message.ordinal)
+    return {isEditing}
+  },
   mapDispatchToProps,
-  (_, dispatchProps, ownProps: OwnProps) => {
+  (stateProps, dispatchProps, ownProps: OwnProps) => {
     const {message} = ownProps
     const {height, width} = Constants.clampImageSize(
       message.previewWidth,
@@ -79,6 +84,8 @@ export default Container.connect(
       height,
       inlineVideoPlayable: message.inlineVideoPlayable,
       isCollapsed: message.isCollapsed,
+      isEditing: stateProps.isEditing,
+      isHighlighted: ownProps.isHighlighted,
       message,
       onClick: () => dispatchProps._onClick(message),
       onCollapse: () => dispatchProps._onCollapse(message),

shared/chat/conversation/messages/attachment/image/index.tsx

@@ -6,14 +6,16 @@ import {isMobile} from '../../../../../util/container'
 import {memoize} from '../../../../../util/memoize'
 import * as Types from '../../../../../constants/types/chat2'
 import * as Constants from '../../../../../constants/chat2'
-import {ShowToastAfterSaving} from '../shared'
+import {getEditStyle, ShowToastAfterSaving} from '../shared'
 
 type Props = {
   arrowColor: string
   downloadError: boolean
   hasProgress: boolean
   height: number
   isCollapsed: boolean
+  isHighlighted?: boolean
+  isEditing: boolean
   onClick: () => void
   onCollapse: () => void
   onShowInFinder?: (e: React.BaseSyntheticEvent) => void
@@ -81,7 +83,11 @@ class ImageAttachment extends React.PureComponent<Props, State> {
     return (
       <>
         <ShowToastAfterSaving transferState={this.props.transferState} />
-        <Kb.Box2 direction="vertical" fullWidth={true}>
+        <Kb.Box2
+          direction="vertical"
+          fullWidth={true}
+          style={getEditStyle(this.props.isEditing, this.props.isHighlighted)}
+        >
           {(!mobileImageFilename || !Styles.isMobile) && (
             <Kb.Box2 direction="horizontal" fullWidth={true} gap="xtiny" style={styles.fileNameContainer}>
               <Kb.Text type="BodyTiny" lineClamp={1}>
@@ -208,6 +214,12 @@ class ImageAttachment extends React.PureComponent<Props, State> {
                         <Kb.Markdown
                           meta={this.memoizedMeta(this.props.message)}
                           selectable={true}
+                          style={getEditStyle(this.props.isEditing, this.props.isHighlighted)}
+                          styleOverride={
+                            Styles.isMobile
+                              ? {paragraph: getEditStyle(this.props.isEditing, this.props.isHighlighted)}
+                              : undefined
+                          }
                           allowFontScaling={true}
                         >
                           {this.props.title}
@@ -235,8 +247,7 @@ class ImageAttachment extends React.PureComponent<Props, State> {
               <Kb.Box style={styles.progressContainer}>
                 {!this.props.onShowInFinder && !this.props.downloadError && (
                   <Kb.Text type="BodySmall" style={styles.progressLabel}>
-                    {progressLabel ||
-                      '\u00A0' /* always show this so we don't change sizes when we're uploading. This is a short term thing, ultimately we should hoist this type of overlay up over the content so it can go away and we won't be left with a gap */}
+                    {progressLabel}
                   </Kb.Text>
                 )}
                 {this.props.hasProgress && <Kb.ProgressBar ratio={this.props.progress} />}

shared/chat/conversation/messages/attachment/shared.tsx

@@ -1,6 +1,8 @@
 import * as React from 'react'
 import * as Kb from '../../../../common-adapters'
 import * as Types from '../../../../constants/types/chat2'
+import * as Styles from '../../../../styles'
+import {sharedStyles} from '../shared-styles'
 import {isMobile} from '../../../../constants/platform'
 
 type Props = {
@@ -27,3 +29,10 @@ export const ShowToastAfterSaving = isMobile
       ) : null
     }
   : () => null
+
+export const getEditStyle = (isEditing: boolean, isHighlighted?: boolean) => {
+  if (isHighlighted) {
+    return Styles.collapseStyles([sharedStyles.sent, sharedStyles.highlighted])
+  }
+  return isEditing ? sharedStyles.sentEditing : sharedStyles.sent
+}

shared/chat/conversation/messages/message-popup/attachment/container.tsx

@@ -88,6 +88,14 @@ export default Container.connect(
         })
       )
     },
+    _onEdit: (message: Types.Message) => {
+      dispatch(
+        Chat2Gen.createMessageSetEditing({
+          conversationIDKey: message.conversationIDKey,
+          ordinal: message.ordinal,
+        })
+      )
+    },
     _onForward: (message: Types.Message) => {
       dispatch(
         RouteTreeGen.createNavigateAppend({
@@ -161,6 +169,7 @@ export default Container.connect(
     const message = ownProps.message
     const yourMessage = message.author === stateProps._you
     const isDeleteable = yourMessage || stateProps._canAdminDelete
+    const isEditable = !!(ownProps.message.isEditable && yourMessage)
     const authorInTeam = stateProps._teamMembers?.has(message.author) ?? true
     return {
       attachTo: ownProps.attachTo,
@@ -169,12 +178,14 @@ export default Container.connect(
       deviceRevokedAt: message.deviceRevokedAt || undefined,
       deviceType: message.deviceType,
       isDeleteable,
+      isEditable,
       isKickable: isDeleteable && !!stateProps._teamID && !yourMessage && authorInTeam,
       onAddReaction: isMobile ? () => dispatchProps._onAddReaction(message) : undefined,
       onAllMedia: () => dispatchProps._onAllMedia(message.conversationIDKey),
       onCopyLink: () => dispatchProps._onCopyLink(stateProps._label, message),
       onDelete: isDeleteable ? () => dispatchProps._onDelete(message) : undefined,
       onDownload: !isMobile && !message.downloadPath ? () => dispatchProps._onDownload(message) : undefined,
+      onEdit: () => dispatchProps._onEdit(message),
       onForward: () => dispatchProps._onForward(message),
       onHidden: () => ownProps.onHidden(),
       onInstallBot: stateProps._authorIsBot ? () => dispatchProps._onInstallBot(message) : undefined,

shared/chat/conversation/messages/message-popup/attachment/index.tsx

@@ -17,6 +17,7 @@ type Props = {
   onCopyLink?: () => void
   onDelete?: () => void
   onDownload?: () => void
+  onEdit: () => void
   onForward: () => void
   onHidden: () => void
   onInstallBot?: () => void
@@ -34,6 +35,7 @@ type Props = {
   visible: boolean
   yourMessage: boolean
   isDeleteable: boolean
+  isEditable: boolean
   isKickable: boolean
 }
 
@@ -89,6 +91,15 @@ const AttachmentPopupMenu = (props: Props) => {
       ? [{icon: 'iconfont-link', onClick: props.onCopyLink, title: 'Copy a link to this message'}]
       : []),
     ...(props.onReply ? [{icon: 'iconfont-reply', onClick: props.onReply, title: 'Reply'}] : []),
+    ...(props.onEdit && props.isEditable
+      ? [
+          {
+            icon: 'iconfont-edit',
+            onClick: props.onEdit,
+            title: 'Edit',
+          },
+        ]
+      : []),
     ...(props.onForward ? [{icon: 'iconfont-forward', onClick: props.onForward, title: 'Forward'}] : []),
     ...(props.onPinMessage
       ? [{icon: 'iconfont-pin', onClick: props.onPinMessage, title: 'Pin message'}]

shared/chat/conversation/messages/shared-styles.tsx

@@ -0,0 +1,46 @@
+import * as Styles from '../../../styles'
+
+const editing = {
+  backgroundColor: Styles.globalColors.yellowLight,
+  borderRadius: 2,
+  color: Styles.globalColors.blackOrBlack,
+  paddingLeft: Styles.globalMargins.tiny,
+  paddingRight: Styles.globalMargins.tiny,
+}
+const sent = Styles.platformStyles({
+  isElectron: {
+    // Make text selectable. On mobile we implement that differently.
+    cursor: 'text',
+    userSelect: 'text',
+    whiteSpace: 'pre-wrap',
+    width: '100%',
+    wordBreak: 'break-word',
+  } as const,
+  isMobile: {
+    ...Styles.globalStyles.flexBoxColumn,
+  },
+})
+const sentEditing = {
+  ...sent,
+  ...editing,
+}
+const pendingFail = {
+  ...sent,
+}
+const pendingFailEditing = {
+  ...pendingFail,
+  ...editing,
+}
+export const sharedStyles = Styles.styleSheetCreate(
+  () =>
+    ({
+      editing,
+      highlighted: {
+        color: Styles.globalColors.blackOrBlack,
+      },
+      pendingFail,
+      pendingFailEditing,
+      sent,
+      sentEditing,
+    } as const)
+)