Improve caption editor usability, new 'add attachment' affordance

ts/components/CaptionEditor.md

@@ -9,7 +9,8 @@ let caption = null;
     attachment={{
       contentType: 'image/jpeg',
     }}
-    onChangeCaption={caption => console.log('onChangeCaption', caption)}
+    onSave={caption => console.log('onSave', caption)}
+    close={() => console.log('close')}
     i18n={util.i18n}
   />
 </div>;
@@ -29,7 +30,8 @@ let caption =
     }}
     caption={caption}
     contentType="image/jpeg"
-    onChangeCaption={caption => console.log('onChangeCaption', caption)}
+    onSave={caption => console.log('onSave', caption)}
+    close={() => console.log('close')}
     i18n={util.i18n}
   />
 </div>;
@@ -46,7 +48,8 @@ let caption = null;
     attachment={{
       contentType: 'video/mp4',
     }}
-    onChangeCaption={caption => console.log('onChangeCaption', caption)}
+    onSave={caption => console.log('onSave', caption)}
+    close={() => console.log('close')}
     i18n={util.i18n}
   />
 </div>;
@@ -65,7 +68,8 @@ let caption =
       contentType: 'video/mp4',
     }}
     caption={caption}
-    onChangeCaption={caption => console.log('onChangeCaption', caption)}
+    onSave={caption => console.log('onSave', caption)}
+    close={() => console.log('close')}
     i18n={util.i18n}
   />
 </div>;

ts/components/CaptionEditor.tsx

@@ -12,31 +12,52 @@ interface Props {
   i18n: Localizer;
   url: string;
   caption?: string;
-  onChangeCaption?: (caption: string) => void;
+  onSave?: (caption: string) => void;
   close?: () => void;
 }
 
-export class CaptionEditor extends React.Component<Props> {
-  private handleKeyUpBound: () => void;
+interface State {
+  caption: string;
+}
+
+export class CaptionEditor extends React.Component<Props, State> {
+  private handleKeyUpBound: (
+    event: React.KeyboardEvent<HTMLInputElement>
+  ) => void;
   private setFocusBound: () => void;
+  // TypeScript doesn't like our React.Ref typing here, so we omit it
   private captureRefBound: () => void;
+  private onChangeBound: () => void;
+  private onSaveBound: () => void;
   private inputRef: React.Ref<HTMLInputElement> | null;
 
   constructor(props: Props) {
     super(props);
 
+    const { caption } = props;
+    this.state = {
+      caption: caption || '',
+    };
+
     this.handleKeyUpBound = this.handleKeyUp.bind(this);
     this.setFocusBound = this.setFocus.bind(this);
     this.captureRefBound = this.captureRef.bind(this);
+    this.onChangeBound = this.onChange.bind(this);
+    this.onSaveBound = this.onSave.bind(this);
     this.inputRef = null;
   }
 
   public handleKeyUp(event: React.KeyboardEvent<HTMLInputElement>) {
-    const { close } = this.props;
+    const { close, onSave } = this.props;
 
-    if (close && (event.key === 'Escape' || event.key === 'Enter')) {
+    if (close && event.key === 'Escape') {
       close();
     }
+
+    if (onSave && event.key === 'Enter') {
+      const { caption } = this.state;
+      onSave(caption);
+    }
   }
 
   public setFocus() {
@@ -55,6 +76,24 @@ export class CaptionEditor extends React.Component<Props> {
     }, 200);
   }
 
+  public onSave() {
+    const { onSave } = this.props;
+    const { caption } = this.state;
+
+    if (onSave) {
+      onSave(caption);
+    }
+  }
+
+  public onChange(event: React.FormEvent<HTMLInputElement>) {
+    // @ts-ignore
+    const { value } = event.target;
+
+    this.setState({
+      caption: value,
+    });
+  }
+
   public renderObject() {
     const { url, i18n, attachment } = this.props;
     const { contentType } = attachment || { contentType: null };
@@ -83,7 +122,8 @@ export class CaptionEditor extends React.Component<Props> {
   }
 
   public render() {
-    const { caption, i18n, close, onChangeCaption } = this.props;
+    const { i18n, close } = this.props;
+    const { caption } = this.state;
 
     return (
       <div
@@ -100,21 +140,27 @@ export class CaptionEditor extends React.Component<Props> {
           {this.renderObject()}
         </div>
         <div className="module-caption-editor__bottom-bar">
-          <div className="module-caption-editor__add-caption-button" />
-          <input
-            type="text"
-            ref={this.captureRefBound}
-            onKeyUp={close ? this.handleKeyUpBound : undefined}
-            value={caption || ''}
-            maxLength={200}
-            placeholder={i18n('addACaption')}
-            className="module-caption-editor__caption-input"
-            onChange={event => {
-              if (onChangeCaption) {
-                onChangeCaption(event.target.value);
-              }
-            }}
-          />
+          <div className="module-caption-editor__input-container">
+            <input
+              type="text"
+              ref={this.captureRefBound}
+              value={caption}
+              maxLength={200}
+              placeholder={i18n('addACaption')}
+              className="module-caption-editor__caption-input"
+              onKeyUp={close ? this.handleKeyUpBound : undefined}
+              onChange={this.onChangeBound}
+            />
+            {caption ? (
+              <div
+                role="button"
+                onClick={this.onSaveBound}
+                className="module-caption-editor__save-button"
+              >
+                {i18n('save')}
+              </div>
+            ) : null}
+          </div>
         </div>
       </div>
     );

ts/components/conversation/AttachmentList.md

@@ -19,8 +19,9 @@ const attachments = [
     onCloseAttachment={attachment => {
       console.log('onCloseAttachment', attachment);
     }}
+    onAddAttachment={() => console.log('onAddAttachment')}
     i18n={util.i18n}
-  />;
+  />
 </util.ConversationContext>;
 ```
 
@@ -64,6 +65,7 @@ const attachments = [
     onCloseAttachment={attachment => {
       console.log('onCloseAttachment', attachment);
     }}
+    onAddAttachment={() => console.log('onAddAttachment')}
     i18n={util.i18n}
   />
 </util.ConversationContext>;
@@ -101,6 +103,7 @@ const attachments = [
     onCloseAttachment={attachment => {
       console.log('onCloseAttachment', attachment);
     }}
+    onAddAttachment={() => console.log('onAddAttachment')}
     i18n={util.i18n}
   />
 </util.ConversationContext>;

ts/components/conversation/AttachmentList.tsx

@@ -6,7 +6,9 @@ import {
 } from '../../util/GoogleChrome';
 import { AttachmentType } from './types';
 import { Image } from './Image';
+import { areAllAttachmentsVisual } from './ImageGrid';
 import { StagedGenericAttachment } from './StagedGenericAttachment';
+import { StagedPlaceholderAttachment } from './StagedPlaceholderAttachment';
 import { Localizer } from '../../types/Util';
 
 interface Props {
@@ -15,6 +17,7 @@ interface Props {
   // onError: () => void;
   onClickAttachment: (attachment: AttachmentType) => void;
   onCloseAttachment: (attachment: AttachmentType) => void;
+  onAddAttachment: () => void;
   onClose: () => void;
 }
 
@@ -27,6 +30,7 @@ export class AttachmentList extends React.Component<Props> {
     const {
       attachments,
       i18n,
+      onAddAttachment,
       onClickAttachment,
       onCloseAttachment,
       onClose,
@@ -36,6 +40,8 @@ export class AttachmentList extends React.Component<Props> {
       return null;
     }
 
+    const allVisualAttachments = areAllAttachmentsVisual(attachments);
+
     return (
       <div className="module-attachments">
         {attachments.length > 1 ? (
@@ -85,6 +91,9 @@ export class AttachmentList extends React.Component<Props> {
               />
             );
           })}
+          {allVisualAttachments ? (
+            <StagedPlaceholderAttachment onClick={onAddAttachment} />
+          ) : null}
         </div>
       </div>
     );

ts/components/conversation/ImageGrid.tsx

@@ -389,7 +389,9 @@ function getImageDimensions(attachment: AttachmentType): DimensionsType {
   };
 }
 
-function areAllAttachmentsVisual(attachments?: Array<AttachmentType>): boolean {
+export function areAllAttachmentsVisual(
+  attachments?: Array<AttachmentType>
+): boolean {
   if (!attachments) {
     return false;
   }
@@ -397,7 +399,7 @@ function areAllAttachmentsVisual(attachments?: Array<AttachmentType>): boolean {
   const max = attachments.length;
   for (let i = 0; i < max; i += 1) {
     const attachment = attachments[i];
-    if (!isImageAttachment(attachment) || !isVideoAttachment(attachment)) {
+    if (!isImageAttachment(attachment) && !isVideoAttachment(attachment)) {
       return false;
     }
   }

ts/components/conversation/StagedPlaceholderAttachment.md

@@ -0,0 +1,10 @@
+```js
+const attachment = {
+  contentType: 'text/plain',
+  fileName: 'manifesto.txt',
+};
+
+<util.ConversationContext theme={util.theme}>
+  <StagedPlaceholderAttachment onClick={attachment => console.log('onClick')} />
+</util.ConversationContext>;
+```

ts/components/conversation/StagedPlaceholderAttachment.tsx

@@ -0,0 +1,21 @@
+import React from 'react';
+
+interface Props {
+  onClick: () => void;
+}
+
+export class StagedPlaceholderAttachment extends React.Component<Props> {
+  public render() {
+    const { onClick } = this.props;
+
+    return (
+      <div
+        className="module-staged-placeholder-attachment"
+        role="button"
+        onClick={onClick}
+      >
+        <div className="module-staged-placeholder-attachment__plus-icon" />
+      </div>
+    );
+  }
+}