From f5226f9d5b51066be04a7f44a5fa5c542ff04a64 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Wed, 2 Feb 2022 09:30:53 +0000 Subject: [PATCH] Simplify Composer buttons (#7678) * Render a CollapsibleButton's children (needed by UploadButton) * Make UploadButton ready to live inside an overflow menu * Always show overflow menu in composer: main buttons are emoji and attach * Re-order composer buttons as per design * Re-word composer button captions to be simple nouns * Don't rotate More options button when clicked * Move the composer menu and dialogs 16px in from right * Reduce shadow on composer More menu * From review: remove else clause * From review: take input out of button * Update test snapshots * Update snapshots --- res/css/structures/_ContextualMenu.scss | 2 +- res/css/views/rooms/_MessageComposer.scss | 9 +- .../views/location/LocationButton.tsx | 4 +- .../views/rooms/CollapsibleButton.tsx | 11 +- .../views/rooms/MessageComposerButtons.tsx | 116 ++++++++---------- src/i18n/strings/en_EN.json | 17 ++- .../rooms/MessageComposerButtons-test.tsx | 49 ++++++-- 7 files changed, 110 insertions(+), 98 deletions(-) diff --git a/res/css/structures/_ContextualMenu.scss b/res/css/structures/_ContextualMenu.scss index 18873978d3..a010f44764 100644 --- a/res/css/structures/_ContextualMenu.scss +++ b/res/css/structures/_ContextualMenu.scss @@ -41,7 +41,7 @@ limitations under the License. } .mx_ContextualMenu_right { - right: 0; + right: 16px; } .mx_ContextualMenu.mx_ContextualMenu_withChevron_right { diff --git a/res/css/views/rooms/_MessageComposer.scss b/res/css/views/rooms/_MessageComposer.scss index c55e803747..3b5ec8365b 100644 --- a/res/css/views/rooms/_MessageComposer.scss +++ b/res/css/views/rooms/_MessageComposer.scss @@ -21,7 +21,7 @@ limitations under the License. border-top: 1px solid $primary-hairline-color; position: relative; padding-left: 42px; - padding-right: 6px; + padding-right: 16px; } .mx_MessageComposer_replaced_wrapper { @@ -271,11 +271,6 @@ limitations under the License. mask-image: url('$(res)/img/image-view/more.svg'); } -.mx_MessageComposer_closeButtonMenu::before { - transform: rotate(90deg); - transform-origin: center; -} - .mx_MessageComposer_sendMessage { cursor: pointer; position: relative; @@ -417,4 +412,6 @@ limitations under the License. min-width: 150px; width: max-content; padding: 5px 10px 5px 0; + box-shadow: 0px 2px 9px rgba(0, 0, 0, 0.25); + border-radius: 8px; } diff --git a/src/components/views/location/LocationButton.tsx b/src/components/views/location/LocationButton.tsx index 815e001717..d3f6e7cb3d 100644 --- a/src/components/views/location/LocationButton.tsx +++ b/src/components/views/location/LocationButton.tsx @@ -72,13 +72,11 @@ export const LocationButton: React.FC = ({ roomId, sender, menuPosition }, ); - // TODO: replace ContextMenuTooltipButton with a unified representation of - // the header buttons and the right panel buttons return { contextMenu } diff --git a/src/components/views/rooms/CollapsibleButton.tsx b/src/components/views/rooms/CollapsibleButton.tsx index a0a802d6b4..b9e9f083d0 100644 --- a/src/components/views/rooms/CollapsibleButton.tsx +++ b/src/components/views/rooms/CollapsibleButton.tsx @@ -25,7 +25,7 @@ interface ICollapsibleButtonProps extends ComponentProps { title: string; } -export const CollapsibleButton = ({ title, className, ...props }: ICollapsibleButtonProps) => { +export const CollapsibleButton = ({ title, children, className, ...props }: ICollapsibleButtonProps) => { const inOverflowMenu = !!useContext(OverflowMenuContext); if (inOverflowMenu) { return { title } + { children } ; } - return ; + return + { children } + ; }; export default CollapsibleButton; diff --git a/src/components/views/rooms/MessageComposerButtons.tsx b/src/components/views/rooms/MessageComposerButtons.tsx index a5ebddde89..b6dcad81e6 100644 --- a/src/components/views/rooms/MessageComposerButtons.tsx +++ b/src/components/views/rooms/MessageComposerButtons.tsx @@ -59,53 +59,47 @@ const MessageComposerButtons: React.FC = (props: IProps) => { const matrixClient: MatrixClient = useContext(MatrixClientContext); const { room, roomId } = useContext(RoomContext); - return ( - props.haveRecording - ? null - : props.narrowMode - ? narrowMode(props, room, roomId, matrixClient) - : wideMode(props, room, roomId, matrixClient) - ); -}; + if (props.haveRecording) { + return null; + } -function wideMode( - props: IProps, - room: Room, - roomId: string, - matrixClient: MatrixClient, -): ReactElement { - return <> - { pollButton(props, room) } - { uploadButton(props, roomId) } - { showLocationButton(props, room, roomId, matrixClient) } - { emojiButton(props) } - { showStickersButton(props) } - { voiceRecordingButton(props) } - ; -} + let mainButtons: ReactElement[]; + let moreButtons: ReactElement[]; + if (props.narrowMode) { + mainButtons = [ + emojiButton(props), + ]; + moreButtons = [ + uploadButton(props, roomId), + showStickersButton(props), + voiceRecordingButton(props), + pollButton(room), + showLocationButton(props, room, roomId, matrixClient), + ]; + } else { + mainButtons = [ + emojiButton(props), + uploadButton(props, roomId), + ]; + moreButtons = [ + showStickersButton(props), + voiceRecordingButton(props), + pollButton(room), + showLocationButton(props, room, roomId, matrixClient), + ]; + } + + mainButtons = mainButtons.filter((x: ReactElement) => x); + moreButtons = moreButtons.filter((x: ReactElement) => x); -function narrowMode( - props: IProps, - room: Room, - roomId: string, - matrixClient: MatrixClient, -): ReactElement { const moreOptionsClasses = classNames({ mx_MessageComposer_button: true, mx_MessageComposer_buttonMenu: true, mx_MessageComposer_closeButtonMenu: props.isMenuOpen, }); - const moreButtons = [ - pollButton(props, room), - showLocationButton(props, room, roomId, matrixClient), - emojiButton(props), - showStickersButton(props), - voiceRecordingButton(props), - ].filter(x => x); - return <> - { uploadButton(props, roomId) } + { mainButtons } ) } ; -} +}; function emojiButton(props: IProps): ReactElement { return = ({ addEmoji, menuPosition }) => { contextMenu } @@ -219,7 +213,7 @@ class UploadButton extends React.Component { dis.dispatch({ action: 'require_registration' }); return; } - this.uploadInput.current.click(); + this.uploadInput.current?.click(); }; private onUploadFileInputChange = (ev: React.ChangeEvent) => { @@ -249,21 +243,20 @@ class UploadButton extends React.Component { render() { const uploadInputStyle = { display: 'none' }; - return ( - + - - - ); + title={_t('Attachment')} + /> + + ; } } @@ -275,13 +268,7 @@ function showStickersButton(props: IProps): ReactElement { key="controls_stickers" className="mx_MessageComposer_button mx_MessageComposer_stickers" onClick={() => props.setStickerPickerOpen(!props.isStickerPickerOpen)} - title={ - props.narrowMode - ? _t("Send a sticker") - : props.isStickerPickerOpen - ? _t("Hide Stickers") - : _t("Show Stickers") - } + title={props.isStickerPickerOpen ? _t("Hide stickers") : _t("Sticker")} /> : null ); @@ -296,12 +283,12 @@ function voiceRecordingButton(props: IProps): ReactElement { key="voice_message_send" className="mx_MessageComposer_button mx_MessageComposer_voiceMessage" onClick={props.onRecordStartEndClick} - title={_t("Send voice message")} + title={_t("Voice Message")} /> ); } -function pollButton(props: IProps, room: Room): ReactElement { +function pollButton(room: Room): ReactElement { return ; } @@ -311,6 +298,7 @@ interface IPollButtonProps { class PollButton extends React.PureComponent { static contextType = OverflowMenuContext; + public context!: React.ContextType; private onCreateClick = () => { this.context?.(); // close overflow menu @@ -350,7 +338,7 @@ class PollButton extends React.PureComponent { ); } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index a4bc83206a..ec1a079592 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1694,13 +1694,12 @@ "You do not have permission to post to this room": "You do not have permission to post to this room", "%(seconds)ss left": "%(seconds)ss left", "Send voice message": "Send voice message", - "Add emoji": "Add emoji", - "Upload file": "Upload file", - "Send a sticker": "Send a sticker", - "Hide Stickers": "Hide Stickers", - "Show Stickers": "Show Stickers", + "Emoji": "Emoji", + "Hide stickers": "Hide stickers", + "Sticker": "Sticker", + "Voice Message": "Voice Message", "You do not have permission to start polls in this room.": "You do not have permission to start polls in this room.", - "Create poll": "Create poll", + "Poll": "Poll", "Bold": "Bold", "Italics": "Italics", "Strikethrough": "Strikethrough", @@ -2095,7 +2094,6 @@ "Invalid file%(extra)s": "Invalid file%(extra)s", "Error decrypting image": "Error decrypting image", "Show image": "Show image", - "Sticker": "Sticker", "Image": "Image", "Join the conference at the top of this room": "Join the conference at the top of this room", "Join the conference from the room information card on the right": "Join the conference from the room information card on the right", @@ -2153,10 +2151,11 @@ "Submit logs": "Submit logs", "Can't load this message": "Can't load this message", "toggle event": "toggle event", - "Share location": "Share location", + "Location": "Location", "We couldn’t send your location": "We couldn’t send your location", "Element could not send your location. Please try again later.": "Element could not send your location. Please try again later.", "Could not fetch location": "Could not fetch location", + "Share location": "Share location", "Element was denied permission to fetch your location. Please allow location access in your browser settings.": "Element was denied permission to fetch your location. Please allow location access in your browser settings.", "Failed to fetch your location. Please try again later.": "Failed to fetch your location. Please try again later.", "Timed out trying to fetch your location. Please try again later.": "Timed out trying to fetch your location. Please try again later.", @@ -2295,6 +2294,7 @@ "%(oneUser)schanged the server ACLs %(count)s times|one": "%(oneUser)schanged the server ACLs", "%(severalUsers)schanged the pinned messages for the room %(count)s times.|other": "%(severalUsers)schanged the pinned messages for the room %(count)s times.", "%(oneUser)schanged the pinned messages for the room %(count)s times.|other": "%(oneUser)schanged the pinned messages for the room %(count)s times.", + "Create poll": "Create poll", "Create Poll": "Create Poll", "Failed to post poll": "Failed to post poll", "Sorry, the poll you tried to create was not posted.": "Sorry, the poll you tried to create was not posted.", @@ -3280,7 +3280,6 @@ "Commands": "Commands", "Command Autocomplete": "Command Autocomplete", "Community Autocomplete": "Community Autocomplete", - "Emoji": "Emoji", "Emoji Autocomplete": "Emoji Autocomplete", "Notify the whole room": "Notify the whole room", "Room Notification": "Room Notification", diff --git a/test/components/views/rooms/MessageComposerButtons-test.tsx b/test/components/views/rooms/MessageComposerButtons-test.tsx index 1fc9323848..78bf64496e 100644 --- a/test/components/views/rooms/MessageComposerButtons-test.tsx +++ b/test/components/views/rooms/MessageComposerButtons-test.tsx @@ -34,23 +34,45 @@ const MessageComposerButtons = TestUtils.wrapInMatrixClientContext( ); describe("MessageComposerButtons", () => { - it("Renders all buttons in wide mode", () => { + it("Renders emoji and upload buttons in wide mode", () => { const buttons = wrapAndRender( {}} />, ); expect(buttonLabels(buttons)).toEqual([ - "Create poll", - "Upload file", - "Share location", - "Add emoji", - "Show Stickers", - "Send voice message", + "Emoji", + "Attachment", + "More options", + ]); + }); + + it("Renders other buttons in menu in wide mode", () => { + const buttons = wrapAndRender( + {}} + />, + ); + + expect(buttonLabels(buttons)).toEqual([ + "Emoji", + "Attachment", + "More options", + [ + "Sticker", + "Voice Message", + "Poll", + "Location", + ], ]); }); @@ -61,11 +83,12 @@ describe("MessageComposerButtons", () => { narrowMode={true} showLocationButton={true} showStickersButton={true} + toggleButtonMenu={() => {}} />, ); expect(buttonLabels(buttons)).toEqual([ - "Upload file", + "Emoji", "More options", ]); }); @@ -82,13 +105,13 @@ describe("MessageComposerButtons", () => { ); expect(buttonLabels(buttons)).toEqual([ - "Upload file", + "Emoji", "More options", [ - "Create poll", - "Share location", - "Add emoji", - "Send a sticker", + "Attachment", + "Sticker", + "Poll", + "Location", ], ]); });