From d64e0b65c46e946ac190c35b4e00b55648fada24 Mon Sep 17 00:00:00 2001 From: Alvaro <110414366+alvaro-signal@users.noreply.github.com> Date: Wed, 25 Jan 2023 16:51:08 -0700 Subject: [PATCH] Switched ForwardMessageModal to use ListTile --- .eslintrc.js | 2 + stylesheets/_modules.scss | 7 + .../components/AddGroupMembersModal.scss | 5 + stylesheets/components/ContactPills.scss | 4 +- .../components/ForwardMessageModal.scss | 5 + ts/components/AddUserToAnotherGroupModal.tsx | 98 +++++++---- ts/components/ConversationList.tsx | 3 + ts/components/ForwardMessageModal.tsx | 10 +- ts/components/LeftPane.stories.tsx | 16 +- ts/components/LeftPane.tsx | 6 +- ts/components/ListTile.tsx | 69 +++++++- ts/components/conversation/Message.tsx | 2 + .../ChooseGroupMembersModal.tsx | 165 +++++++++++++----- .../BaseConversationListItem.tsx | 2 +- .../conversationList/ContactCheckbox.tsx | 61 ++++--- .../conversationList/ContactListItem.tsx | 53 +++--- .../conversationList/CreateNewGroupButton.tsx | 26 +-- .../conversationList/GroupListItem.tsx | 35 ++-- .../conversationList/PhoneNumberCheckbox.tsx | 61 +++++-- .../conversationList/UsernameCheckbox.tsx | 47 +++-- .../leftPane/LeftPaneComposeHelper.tsx | 10 +- ts/state/selectors/conversations.ts | 21 ++- .../helpers/getDefaultConversation.ts | 15 +- ts/test-mock/pnp/send_gv2_invite_test.ts | 10 +- .../leftPane/LeftPaneComposeHelper_test.ts | 34 ++-- 25 files changed, 528 insertions(+), 239 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index b1bf52c27d2d..3cfd9896848a 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -122,6 +122,8 @@ const rules = { 'react/display-name': 'error', + 'react/jsx-pascal-case': ['error', {allowNamespace: true}], + // Allow returning values from promise executors for brevity. 'no-promise-executor-return': 'off', diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss index e6e2d78ab12b..a6d4175047b7 100644 --- a/stylesheets/_modules.scss +++ b/stylesheets/_modules.scss @@ -4405,6 +4405,13 @@ button.module-image__border-overlay:focus { padding-left: 10px; padding-right: 10px; + // list tiles in choose-group-members and compose extend to the edge + .module-left-pane--mode-choose-group-members &, + .module-left-pane--mode-compose & { + padding-left: 0; + padding-right: 0; + } + &--width-narrow { padding-left: 10px; padding-right: 10px; diff --git a/stylesheets/components/AddGroupMembersModal.scss b/stylesheets/components/AddGroupMembersModal.scss index 687d854bfea8..7648cdad3fc7 100644 --- a/stylesheets/components/AddGroupMembersModal.scss +++ b/stylesheets/components/AddGroupMembersModal.scss @@ -64,4 +64,9 @@ padding: 0; } } + + .module-conversation-list { + // remove horizontal padding so ListTiles extend to the edges + padding: 0; + } } diff --git a/stylesheets/components/ContactPills.scss b/stylesheets/components/ContactPills.scss index a8f8275920c1..ab12b310f34f 100644 --- a/stylesheets/components/ContactPills.scss +++ b/stylesheets/components/ContactPills.scss @@ -10,10 +10,10 @@ max-height: 88px; overflow-x: hidden; overflow-y: scroll; - padding-left: 12px; + padding: 4px 24px; + gap: 8px 12px; .module-ContactPill { - margin: 4px 6px; max-width: calc( 100% - 15px ); // 6px for the right margin and 9px for the scrollbar diff --git a/stylesheets/components/ForwardMessageModal.scss b/stylesheets/components/ForwardMessageModal.scss index 775e9f665226..a473724a4e3f 100644 --- a/stylesheets/components/ForwardMessageModal.scss +++ b/stylesheets/components/ForwardMessageModal.scss @@ -22,6 +22,11 @@ color: $color-gray-05; } + .module-conversation-list { + // remove horizontal padding so ListTiles extend to the edges + padding: 0; + } + &--link-preview { border-bottom: 1px solid $color-gray-15; padding: 12px 16px; diff --git a/ts/components/AddUserToAnotherGroupModal.tsx b/ts/components/AddUserToAnotherGroupModal.tsx index 16140b3e0bbb..cb4c145aca4a 100644 --- a/ts/components/AddUserToAnotherGroupModal.tsx +++ b/ts/components/AddUserToAnotherGroupModal.tsx @@ -1,10 +1,11 @@ // Copyright 2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import { noop, pick } from 'lodash'; -import React from 'react'; +import { pick } from 'lodash'; +import React, { useCallback } from 'react'; import type { MeasuredComponentProps } from 'react-measure'; import Measure from 'react-measure'; +import type { ListRowProps } from 'react-virtualized'; import type { ConversationType } from '../state/ducks/conversations'; import type { @@ -15,12 +16,16 @@ import type { import { ToastType } from '../types/Toast'; import { filterAndSortConversationsByRecent } from '../util/filterAndSortConversations'; import { ConfirmationDialog } from './ConfirmationDialog'; -import type { Row } from './ConversationList'; -import { ConversationList, RowType } from './ConversationList'; -import { DisabledReason } from './conversationList/GroupListItem'; +import type { GroupListItemConversationType } from './conversationList/GroupListItem'; +import { + DisabledReason, + GroupListItem, +} from './conversationList/GroupListItem'; import { Modal } from './Modal'; import { SearchInput } from './SearchInput'; import { useRestoreFocus } from '../hooks/useRestoreFocus'; +import { ListView } from './ListView'; +import { ListTile } from './ListTile'; type OwnProps = { i18n: LocalizerType; @@ -47,7 +52,6 @@ export type Props = OwnProps & DispatchProps; export function AddUserToAnotherGroupModal({ i18n, - theme, contact, toggleAddUserToAnotherGroupModal, addMembersToGroup, @@ -108,7 +112,7 @@ export function AddUserToAnotherGroupModal({ ); const handleGetRow = React.useCallback( - (idx: number): Row | undefined => { + (idx: number): GroupListItemConversationType => { const convo = filteredConversations[idx]; // these are always populated in the case of a group @@ -129,18 +133,36 @@ export function AddUserToAnotherGroupModal({ } return { - type: RowType.SelectSingleGroup, - group: { - ...pick(convo, 'id', 'avatarPath', 'title', 'unblurredAvatarPath'), - memberships, - membersCount, - disabledReason, - }, + ...pick(convo, 'id', 'avatarPath', 'title', 'unblurredAvatarPath'), + memberships, + membersCount, + disabledReason, }; }, [filteredConversations, contact] ); + const renderGroupListItem = useCallback( + ({ key, index, style }: ListRowProps) => { + const group = handleGetRow(index); + return ( +
+ +
+ ); + }, + [i18n, handleGetRow] + ); + + const handleCalculateRowHeight = useCallback( + () => ListTile.heightCompact, + [] + ); + return ( <> {!selectedGroup && ( @@ -163,30 +185,30 @@ export function AddUserToAnotherGroupModal({ /> - {({ contentRect, measureRef }: MeasuredComponentProps) => ( -
- undefined} - getRow={handleGetRow} - i18n={i18n} - lookupConversationWithoutUuid={async _ => undefined} - onClickArchiveButton={noop} - onClickContactCheckbox={noop} - onSelectConversation={setSelectedGroupId} - rowCount={filteredConversations.length} - setIsFetchingUUID={noop} - shouldRecomputeRowHeights={false} - showChooseGroupMembers={noop} - showConversation={noop} - showUserNotFoundModal={noop} - theme={theme} - /> -
- )} + {({ contentRect, measureRef }: MeasuredComponentProps) => { + // Though `width` and `height` are required properties, we want to be + // careful in case the caller sends bogus data. Notably, react-measure's + // types seem to be inaccurate. + const { width = 100, height = 100 } = contentRect.bounds || {}; + if (!width || !height) { + return null; + } + + return ( +
+ +
+ ); + }} diff --git a/ts/components/ConversationList.tsx b/ts/components/ConversationList.tsx index f2c0cb44a743..b94b264fb43d 100644 --- a/ts/components/ConversationList.tsx +++ b/ts/components/ConversationList.tsx @@ -215,6 +215,9 @@ export function ConversationList({ case RowType.SearchResultsLoadingFakeHeader: return HEADER_ROW_HEIGHT; case RowType.SelectSingleGroup: + case RowType.ContactCheckbox: + case RowType.Contact: + case RowType.CreateNewGroup: return SELECT_ROW_HEIGHT; default: return NORMAL_ROW_HEIGHT; diff --git a/ts/components/ForwardMessageModal.tsx b/ts/components/ForwardMessageModal.tsx index fc6795b496dd..9d471e714dd6 100644 --- a/ts/components/ForwardMessageModal.tsx +++ b/ts/components/ForwardMessageModal.tsx @@ -38,6 +38,7 @@ import { shouldNeverBeCalled, asyncShouldNeverBeCalled, } from '../util/shouldNeverBeCalled'; +import { Emojify } from './conversation/Emojify'; export type DataPropsType = { attachments?: ReadonlyArray; @@ -408,8 +409,13 @@ export function ForwardMessageModal({ )}
- {Boolean(selectedContacts.length) && - selectedContacts.map(contact => contact.title).join(', ')} + {Boolean(selectedContacts.length) && ( + contact.title) + .join(', ')} + /> + )}
{isEditingMessage || !isMessageEditable ? ( diff --git a/ts/components/LeftPane.stories.tsx b/ts/components/LeftPane.stories.tsx index 85db2782d91c..e426509715a9 100644 --- a/ts/components/LeftPane.stories.tsx +++ b/ts/components/LeftPane.stories.tsx @@ -23,14 +23,18 @@ import { setupI18n } from '../util/setupI18n'; import { DurationInSeconds, DAY } from '../util/durations'; import enMessages from '../../_locales/en/messages.json'; import { ThemeType } from '../types/Util'; +import { + getDefaultConversation, + getDefaultGroupListItem, +} from '../test-both/helpers/getDefaultConversation'; import { DialogType } from '../types/Dialogs'; import { SocketStatus } from '../types/SocketStatus'; -import { getDefaultConversation } from '../test-both/helpers/getDefaultConversation'; import { StorybookThemeContext } from '../../.storybook/StorybookThemeContext'; import { makeFakeLookupConversationWithoutUuid, useUuidFetchState, } from '../test-both/helpers/fakeLookupConversationWithoutUuid'; +import type { GroupListItemConversationType } from './conversationList/GroupListItem'; const i18n = setupI18n('en', enMessages); @@ -62,18 +66,14 @@ const defaultSearchProps = { startSearchCounter: 0, }; -const defaultGroups: Array = [ - getDefaultConversation({ +const defaultGroups: Array = [ + getDefaultGroupListItem({ id: 'biking-group', title: 'Mtn Biking Arizona 🚵☀️⛰', - type: 'group', - sharedGroupNames: [], }), - getDefaultConversation({ + getDefaultGroupListItem({ id: 'dance-group', title: 'Are we dancers? 💃', - type: 'group', - sharedGroupNames: [], }), ]; diff --git a/ts/components/LeftPane.tsx b/ts/components/LeftPane.tsx index 741ab8bf6f83..17e5fed9fd87 100644 --- a/ts/components/LeftPane.tsx +++ b/ts/components/LeftPane.tsx @@ -605,7 +605,11 @@ export function LeftPane({ className={classNames( 'module-left-pane', isResizing && 'module-left-pane--is-resizing', - `module-left-pane--width-${widthBreakpoint}` + `module-left-pane--width-${widthBreakpoint}`, + modeSpecificProps.mode === LeftPaneMode.ChooseGroupMembers && + 'module-left-pane--mode-choose-group-members', + modeSpecificProps.mode === LeftPaneMode.Compose && + 'module-left-pane--mode-compose' )} style={{ width }} > diff --git a/ts/components/ListTile.tsx b/ts/components/ListTile.tsx index cf1a275653f5..2b07e33ff965 100644 --- a/ts/components/ListTile.tsx +++ b/ts/components/ListTile.tsx @@ -2,8 +2,10 @@ // SPDX-License-Identifier: AGPL-3.0-only import classNames from 'classnames'; -import React from 'react'; +import React, { useMemo } from 'react'; +import uuid from 'uuid'; import { getClassNamesFor } from '../util/getClassNamesFor'; +import { CircleCheckbox } from './CircleCheckbox'; export type Props = { title: string | JSX.Element; @@ -23,6 +25,7 @@ export type Props = { variant?: 'item' | 'panelrow'; // defaults to div rootElement?: 'div' | 'button'; + testId?: string; }; const getClassName = getClassNamesFor('ListTile'); @@ -53,8 +56,17 @@ const getClassName = getClassNamesFor('ListTile'); * - panelrow: more horizontal padding, intended for information rows (usually not in * modals) that tend to occupy more horizontal space */ -export const ListTile = React.forwardRef( - function ListTile( +export function ListTile( + params: Props & React.RefAttributes +): JSX.Element { + // forwardRef makes it impossible to add extra static fields to the function type so + // we have to create this inner implementation that can be wrapped with a non-arrow + // function. A bit weird, but looks fine at call-site. + return ; +} + +const ListTileImpl = React.forwardRef( + function ListTileImpl( { title, subtitle, @@ -67,6 +79,7 @@ export const ListTile = React.forwardRef( disabled = false, variant = 'item', rootElement = 'div', + testId, }: Props, ref ) { @@ -81,6 +94,7 @@ export const ListTile = React.forwardRef( onClick, 'aria-disabled': disabled ? true : undefined, onContextMenu, + 'data-testid': testId, }; const contents = ( @@ -112,3 +126,52 @@ export const ListTile = React.forwardRef( ); } ); + +// although these heights are not required for ListTile (which sizes itself based on +// content), they are useful as constants for ListView.calculateRowHeight + +/** Effective ListTile height for an avatar (leading) size 36 */ +ListTile.heightFull = 64; + +/** Effective ListTile height for an avatar (leading) size 48 */ +ListTile.heightCompact = 52; + +/** + * ListTile with a trailing checkbox. + * + * It also wraps the ListTile with a