From 37992715cd639ce111f1e7fd5af39340f205f5dc Mon Sep 17 00:00:00 2001 From: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> Date: Tue, 9 Nov 2021 18:25:29 -0600 Subject: [PATCH] Remove caption editor --- _locales/en/messages.json | 6 +- js/modules/signal.js | 2 - stylesheets/_modules.scss | 123 ------------ ts/background.ts | 9 +- ts/components/CaptionEditor.stories.tsx | 93 --------- ts/components/CaptionEditor.tsx | 181 ------------------ ts/components/CompositionArea.stories.tsx | 1 - ts/components/CompositionArea.tsx | 3 - ts/components/conversation/AttachmentList.tsx | 5 +- ts/state/smart/ConversationView.tsx | 1 - ts/types/Attachment.ts | 4 +- ts/util/lint/exceptions.json | 16 -- ts/views/conversation_view.ts | 73 +------ ts/window.d.ts | 2 - 14 files changed, 7 insertions(+), 512 deletions(-) delete mode 100644 ts/components/CaptionEditor.stories.tsx delete mode 100644 ts/components/CaptionEditor.tsx diff --git a/_locales/en/messages.json b/_locales/en/messages.json index ba6b6fc58..37e9e2394 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -1268,13 +1268,9 @@ "message": "Icon showing that this image has a caption", "description": "Used for the icon layered on top of an image in message bubbles" }, - "addACaption": { - "message": "Add a caption...", - "description": "Used as the placeholder text in the caption editor text field" - }, "save": { "message": "Save", - "description": "Used as a 'commit changes' button in the caption editor for outgoing image attachments" + "description": "Used on save buttons" }, "reset": { "message": "Reset", diff --git a/js/modules/signal.js b/js/modules/signal.js index 3fa5bc4d0..936a7e4ec 100644 --- a/js/modules/signal.js +++ b/js/modules/signal.js @@ -24,7 +24,6 @@ const Util = require('../../ts/util'); const { AttachmentList, } = require('../../ts/components/conversation/AttachmentList'); -const { CaptionEditor } = require('../../ts/components/CaptionEditor'); const { ChatColorPicker } = require('../../ts/components/ChatColorPicker'); const { ConfirmationDialog, @@ -329,7 +328,6 @@ exports.setup = (options = {}) => { const Components = { AttachmentList, - CaptionEditor, ChatColorPicker, ConfirmationDialog, ContactDetail, diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss index 34c77a2c9..4a76d08cd 100644 --- a/stylesheets/_modules.scss +++ b/stylesheets/_modules.scss @@ -3326,129 +3326,6 @@ button.module-image__border-overlay:focus { text-overflow: ellipsis; } -// Module: Caption Editor - -.module-caption-editor { - background-color: $color-black; - z-index: 20; - - position: absolute; - left: 0; - right: 0; - top: 0; - bottom: 0; - - display: flex; - flex-direction: column; - height: 100%; -} - -.module-caption-editor__close-button { - z-index: 21; - cursor: pointer; - position: absolute; - - top: 12px; - right: 16px; - width: 30px; - height: 30px; - z-index: 2; - @include color-svg('../images/icons/v2/x-24.svg', $color-white); -} - -.module-caption-editor__media-container { - flex-grow: 1; - flex-shrink: 1; - background-color: $color-black; - text-align: center; - margin: 50px; - overflow: hidden; - height: 100%; -} - -.module-caption-editor__image { - width: 100%; - height: 100%; - object-fit: contain; - - flex-grow: 1; - flex-shrink: 1; -} -.module-caption-editor__video { - max-width: 100%; - max-height: 100%; - object-fit: contain; - - flex-grow: 1; - flex-shrink: 1; -} -.module-caption-editor__placeholder { - width: 100%; - height: 100%; - object-fit: contain; - - flex-grow: 1; - flex-shrink: 1; -} - -.module-caption-editor__bottom-bar { - flex-grow: 0; - flex-shrink: 0; - height: 52px; - padding: 8px; - - display: inline-flex; - flex-direction: row; - align-items: middle; - - margin-left: auto; - margin-right: auto; -} - -.module-caption-editor__input-container { - position: relative; -} - -.module-caption-editor__caption-input { - height: 36px; - width: 40em; - - color: $color-white; - - border: 1px solid $color-white; - border-radius: 18px; - background-color: $color-black; - padding: 9px; - padding-left: 12px; - padding-right: 65px; - - &:placeholder { - color: $color-white-alpha-80; - } - &:focus { - border: 1px solid $color-ultramarine; - outline: none; - } -} - -.module-caption-editor__save-button { - @include button-reset; - - position: absolute; - background-color: $color-ultramarine; - color: $color-white; - - height: 28px; - border-radius: 15px; - - padding: 5px; - padding-left: 12px; - padding-right: 12px; - - right: 4px; - top: 4px; -} - // Module: Staged Placeholder Attachment .module-staged-placeholder-attachment { diff --git a/ts/background.ts b/ts/background.ts index 11d2f7b5b..ee06b503f 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -1246,14 +1246,7 @@ export async function startApp(): Promise { const className = (target.attributes as any).class.value; /* eslint-enable @typescript-eslint/no-explicit-any */ - // These want to handle events internally - - // CaptionEditor text box - if (className.includes('module-caption-editor__caption-input')) { - return; - } - - // Search box + // Search box wants to handle events internally if (className.includes('LeftPaneSearchInput__input')) { return; } diff --git a/ts/components/CaptionEditor.stories.tsx b/ts/components/CaptionEditor.stories.tsx deleted file mode 100644 index 61a455d31..000000000 --- a/ts/components/CaptionEditor.stories.tsx +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright 2020 Signal Messenger, LLC -// SPDX-License-Identifier: AGPL-3.0-only - -import * as React from 'react'; - -import { storiesOf } from '@storybook/react'; -import { text } from '@storybook/addon-knobs'; -import { action } from '@storybook/addon-actions'; - -import type { Props } from './CaptionEditor'; -import { CaptionEditor } from './CaptionEditor'; -import { AUDIO_MP3, IMAGE_JPEG, VIDEO_MP4 } from '../types/MIME'; -import { setupI18n } from '../util/setupI18n'; -import enMessages from '../../_locales/en/messages.json'; - -import { fakeAttachment } from '../test-both/helpers/fakeAttachment'; - -const i18n = setupI18n('en', enMessages); - -const stories = storiesOf('Components/Caption Editor', module); - -const createProps = (overrideProps: Partial = {}): Props => ({ - attachment: fakeAttachment({ - contentType: IMAGE_JPEG, - fileName: '', - url: '', - ...overrideProps.attachment, - }), - caption: text('caption', overrideProps.caption || ''), - close: action('close'), - i18n, - onSave: action('onSave'), - url: text('url', overrideProps.url || ''), -}); - -stories.add('Image', () => { - const props = createProps({ - url: '/fixtures/tina-rolf-269345-unsplash.jpg', - }); - - return ; -}); - -stories.add('Image with Caption', () => { - const props = createProps({ - caption: - 'This is the user-provided caption. We show it overlaid on the image. If it is really long, then it wraps, but it does not get too close to the edges of the image.', - url: '/fixtures/tina-rolf-269345-unsplash.jpg', - }); - - return ; -}); - -stories.add('Video', () => { - const props = createProps({ - attachment: fakeAttachment({ - contentType: VIDEO_MP4, - fileName: 'pixabay-Soap-Bubble-7141.mp4', - url: '/fixtures/pixabay-Soap-Bubble-7141.mp4', - }), - url: '/fixtures/pixabay-Soap-Bubble-7141.mp4', - }); - - return ; -}); - -stories.add('Video with Caption', () => { - const props = createProps({ - attachment: fakeAttachment({ - contentType: VIDEO_MP4, - fileName: 'pixabay-Soap-Bubble-7141.mp4', - url: '/fixtures/pixabay-Soap-Bubble-7141.mp4', - }), - caption: - 'This is the user-provided caption. We show it overlaid on the image. If it is really long, then it wraps, but it does not get too close to the edges of the image.', - url: '/fixtures/pixabay-Soap-Bubble-7141.mp4', - }); - - return ; -}); - -stories.add('Unsupported Attachment Type', () => { - const props = createProps({ - attachment: fakeAttachment({ - contentType: AUDIO_MP3, - fileName: 'incompetech-com-Agnus-Dei-X.mp3', - url: '/fixtures/incompetech-com-Agnus-Dei-X.mp3', - }), - url: '/fixtures/incompetech-com-Agnus-Dei-X.mp3', - }); - - return ; -}); diff --git a/ts/components/CaptionEditor.tsx b/ts/components/CaptionEditor.tsx deleted file mode 100644 index 938187439..000000000 --- a/ts/components/CaptionEditor.tsx +++ /dev/null @@ -1,181 +0,0 @@ -// Copyright 2018-2021 Signal Messenger, LLC -// SPDX-License-Identifier: AGPL-3.0-only - -import React from 'react'; -import * as GoogleChrome from '../util/GoogleChrome'; - -import type { AttachmentType } from '../types/Attachment'; - -import type { LocalizerType } from '../types/Util'; - -export type Props = { - attachment: AttachmentType; - i18n: LocalizerType; - url: string; - caption?: string; - onSave?: (caption: string) => void; - close?: () => void; -}; - -type State = { - caption: string; -}; - -export class CaptionEditor extends React.Component { - private readonly handleKeyDownBound: ( - event: React.KeyboardEvent - ) => void; - - private readonly setFocusBound: () => void; - - private readonly onChangeBound: ( - event: React.FormEvent - ) => void; - - private readonly onSaveBound: () => void; - - private readonly inputRef: React.RefObject; - - constructor(props: Props) { - super(props); - - const { caption } = props; - this.state = { - caption: caption || '', - }; - - this.handleKeyDownBound = this.handleKeyDown.bind(this); - this.setFocusBound = this.setFocus.bind(this); - this.onChangeBound = this.onChange.bind(this); - this.onSaveBound = this.onSave.bind(this); - this.inputRef = React.createRef(); - } - - public componentDidMount(): void { - // Forcing focus after a delay due to some focus contention with ConversationView - setTimeout(() => { - this.setFocus(); - }, 200); - } - - public handleKeyDown(event: React.KeyboardEvent): void { - const { close, onSave } = this.props; - - if (close && event.key === 'Escape') { - close(); - - event.stopPropagation(); - event.preventDefault(); - } - - if (onSave && event.key === 'Enter') { - const { caption } = this.state; - onSave(caption); - - event.stopPropagation(); - event.preventDefault(); - } - } - - public setFocus(): void { - if (this.inputRef.current) { - this.inputRef.current.focus(); - } - } - - public onSave(): void { - const { onSave } = this.props; - const { caption } = this.state; - - if (onSave) { - onSave(caption); - } - } - - public onChange(event: React.FormEvent): void { - const { value } = event.target as HTMLInputElement; - - this.setState({ - caption: value, - }); - } - - public renderObject(): JSX.Element { - const { url, i18n, attachment } = this.props; - const { contentType } = attachment || { contentType: null }; - - const isImageTypeSupported = GoogleChrome.isImageTypeSupported(contentType); - if (isImageTypeSupported) { - return ( - {i18n('imageAttachmentAlt')} - ); - } - - const isVideoTypeSupported = GoogleChrome.isVideoTypeSupported(contentType); - if (isVideoTypeSupported) { - return ( - - ); - } - - return
; - } - - // Events handled by props - /* eslint-disable jsx-a11y/click-events-have-key-events */ - public render(): JSX.Element { - const { i18n, close } = this.props; - const { caption } = this.state; - const onKeyDown = close ? this.handleKeyDownBound : undefined; - - return ( -
-
-
- {this.renderObject()} -
-
-
- - {caption ? ( - - ) : null} -
-
-
- ); - } - /* eslint-enable jsx-a11y/click-events-have-key-events */ -} diff --git a/ts/components/CompositionArea.stories.tsx b/ts/components/CompositionArea.stories.tsx index 2d57ff2fb..6ca0f97d0 100644 --- a/ts/components/CompositionArea.stories.tsx +++ b/ts/components/CompositionArea.stories.tsx @@ -37,7 +37,6 @@ const createProps = (overrideProps: Partial = {}): Props => ({ // AttachmentList draftAttachments: overrideProps.draftAttachments || [], onClearAttachments: action('onClearAttachments'), - onClickAttachment: action('onClickAttachment'), // AudioCapture cancelRecording: action('cancelRecording'), completeRecording: action('completeRecording'), diff --git a/ts/components/CompositionArea.tsx b/ts/components/CompositionArea.tsx index c4ab4bafe..c44bba2aa 100644 --- a/ts/components/CompositionArea.tsx +++ b/ts/components/CompositionArea.tsx @@ -97,7 +97,6 @@ export type OwnProps = Readonly<{ linkPreviewResult?: LinkPreviewWithDomain; messageRequestsEnabled?: boolean; onClearAttachments(): unknown; - onClickAttachment(att: AttachmentType): unknown; onClickQuotedMessage(): unknown; onCloseLinkPreview(): unknown; processAttachments: (options: HandleAttachmentsProcessingArgsType) => unknown; @@ -170,7 +169,6 @@ export const CompositionArea = ({ // AttachmentList draftAttachments, onClearAttachments, - onClickAttachment, // AudioCapture cancelRecording, completeRecording, @@ -603,7 +601,6 @@ export const CompositionArea = ({ attachments={draftAttachments} i18n={i18n} onAddAttachment={launchAttachmentPicker} - onClickAttachment={onClickAttachment} onClose={onClearAttachments} onCloseAttachment={attachment => { if (attachment.path) { diff --git a/ts/components/conversation/AttachmentList.tsx b/ts/components/conversation/AttachmentList.tsx index d6da9b31b..1d767972c 100644 --- a/ts/components/conversation/AttachmentList.tsx +++ b/ts/components/conversation/AttachmentList.tsx @@ -67,9 +67,6 @@ export const AttachmentList = ({ const isVideo = isVideoAttachment(attachment); if (isImage || isVideo || attachment.pending) { - const clickCallback = - attachments.length > 1 ? onClickAttachment : undefined; - const imageUrl = url || (isVideo ? BLANK_VIDEO_THUMBNAIL : undefined); @@ -88,7 +85,7 @@ export const AttachmentList = ({ width={IMAGE_WIDTH} url={imageUrl} closeButton - onClick={clickCallback} + onClick={onClickAttachment} onClickClose={onCloseAttachment} onError={() => { onCloseAttachment(attachment); diff --git a/ts/state/smart/ConversationView.tsx b/ts/state/smart/ConversationView.tsx index cf19c4fe9..43533a7fc 100644 --- a/ts/state/smart/ConversationView.tsx +++ b/ts/state/smart/ConversationView.tsx @@ -27,7 +27,6 @@ export type PropsType = { | 'onCancelJoinRequest' | 'onClearAttachments' | 'onClickAddPack' - | 'onClickAttachment' | 'onCloseLinkPreview' | 'onDelete' | 'onEditorStateChange' diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts index 79d14d625..84095e688 100644 --- a/ts/types/Attachment.ts +++ b/ts/types/Attachment.ts @@ -103,8 +103,10 @@ export type AttachmentDraftType = | ({ url: string; screenshotPath?: string; - caption?: string; pending: false; + // Old draft attachments may have a caption, though they are no longer editable + // because we removed the caption editor. + caption?: string; } & BaseAttachmentDraftType) | { contentType: MIME.MIMEType; diff --git a/ts/util/lint/exceptions.json b/ts/util/lint/exceptions.json index 7dcb4edc4..f1adaa22b 100644 --- a/ts/util/lint/exceptions.json +++ b/ts/util/lint/exceptions.json @@ -10303,22 +10303,6 @@ "reasonCategory": "usageTrusted", "updated": "2021-07-30T16:57:33.618Z" }, - { - "rule": "React-createRef", - "path": "ts/components/CaptionEditor.js", - "line": " this.inputRef = react_1.default.createRef();", - "reasonCategory": "usageTrusted", - "updated": "2019-03-09T00:08:44.242Z", - "reasonDetail": "Used only to set focus" - }, - { - "rule": "React-createRef", - "path": "ts/components/CaptionEditor.tsx", - "line": " this.inputRef = React.createRef();", - "reasonCategory": "usageTrusted", - "updated": "2019-03-09T00:08:44.242Z", - "reasonDetail": "Used only to set focus" - }, { "rule": "React-useRef", "path": "ts/components/ChatColorPicker.tsx", diff --git a/ts/views/conversation_view.ts b/ts/views/conversation_view.ts index f4881d441..66b66f4fa 100644 --- a/ts/views/conversation_view.ts +++ b/ts/views/conversation_view.ts @@ -7,7 +7,7 @@ import { batch as batchDispatch } from 'react-redux'; import { debounce, flatten, omit, throttle } from 'lodash'; import { render } from 'mustache'; -import type { AttachmentDraftType, AttachmentType } from '../types/Attachment'; +import type { AttachmentType } from '../types/Attachment'; import { isGIF } from '../types/Attachment'; import * as Attachment from '../types/Attachment'; import type { StickerPackType as StickerPackDBType } from '../sql/Interface'; @@ -239,7 +239,6 @@ export class ConversationView extends window.Backbone.View { private preview?: Array; // Sub-views - private captionEditorView?: Backbone.View; private contactModalView?: Backbone.View; private conversationView?: BasicReactWrapperViewClass; private forwardMessageModal?: Backbone.View; @@ -730,7 +729,6 @@ export class ConversationView extends window.Backbone.View { }); }, - onClickAttachment: this.onClickAttachment.bind(this), onClearAttachments: this.clearAttachments.bind(this), onSelectMediaQuality: (isHQ: boolean) => { window.reduxActions.composer.setMediaQualitySetting(isHQ); @@ -1364,9 +1362,6 @@ export class ConversationView extends window.Backbone.View { this.conversationView?.remove(); - if (this.captionEditorView) { - this.captionEditorView.remove(); - } if (this.contactModalView) { this.contactModalView.remove(); } @@ -1474,72 +1469,6 @@ export class ConversationView extends window.Backbone.View { }); } - onClickAttachment(attachment: AttachmentDraftType): void { - if (attachment.pending) { - throw new Error( - 'onClickAttachment: Cannot click to edit pending attachment' - ); - } - - const getProps = () => { - if (attachment.pending) { - throw new Error( - 'onClickAttachment/onSave: Cannot render pending attachment' - ); - } - - return { - url: attachment.url, - caption: attachment.caption, - attachment, - onSave, - }; - }; - - const onSave = (caption?: string) => { - const attachments = this.model.get('draftAttachments') || []; - - this.model.set({ - draftAttachments: attachments.map((item: AttachmentType) => { - if (item.pending || attachment.pending) { - return item; - } - - if ( - (item.path && item.path === attachment.path) || - (item.screenshotPath && - item.screenshotPath === attachment.screenshotPath) - ) { - return { - ...attachment, - caption, - }; - } - - return item; - }), - draftChanged: true, - }); - - if (this.captionEditorView) { - this.captionEditorView.remove(); - this.captionEditorView = undefined; - } - window.Signal.Backbone.Views.Lightbox.hide(); - - this.updateAttachmentsView(); - this.saveModel(); - }; - - this.captionEditorView = new Whisper.ReactWrapperView({ - className: 'attachment-list-wrapper', - Component: window.Signal.Components.CaptionEditor, - props: getProps(), - onClose: () => window.Signal.Backbone.Views.Lightbox.hide(), - }); - window.Signal.Backbone.Views.Lightbox.show(this.captionEditorView.el); - } - async saveModel(): Promise { window.Signal.Data.updateConversation(this.model.attributes); } diff --git a/ts/window.d.ts b/ts/window.d.ts index a4555209f..e440c31b1 100644 --- a/ts/window.d.ts +++ b/ts/window.d.ts @@ -79,7 +79,6 @@ import { ConversationModel } from './models/conversations'; import { combineNames } from './util'; import { BatcherType } from './util/batcher'; import { AttachmentList } from './components/conversation/AttachmentList'; -import { CaptionEditor } from './components/CaptionEditor'; import { ChatColorPicker } from './components/ChatColorPicker'; import { ConfirmationDialog } from './components/ConfirmationDialog'; import { ContactDetail } from './components/conversation/ContactDetail'; @@ -389,7 +388,6 @@ declare global { }; Components: { AttachmentList: typeof AttachmentList; - CaptionEditor: typeof CaptionEditor; ChatColorPicker: typeof ChatColorPicker; ConfirmationDialog: typeof ConfirmationDialog; ContactDetail: typeof ContactDetail;