From bd46e3afd6f4fd945d57e8aca0aa08bd876e4703 Mon Sep 17 00:00:00 2001 From: Josh Perez <60019601+josh-signal@users.noreply.github.com> Date: Wed, 2 Jun 2021 17:05:09 -0400 Subject: [PATCH] Fixes global chat color setting --- _locales/en/messages.json | 8 +++ ts/background.ts | 6 ++ ts/components/ChatColorPicker.stories.tsx | 1 + ts/components/ChatColorPicker.tsx | 49 ++++++++++--- ts/components/GlobalModalContainer.tsx | 10 +-- ts/models/conversations.ts | 36 ++++++++-- ts/models/messages.ts | 2 +- ts/shims/reloadSelectedConversation.ts | 21 ++++++ ts/state/ducks/conversations.ts | 71 ++++++++----------- ts/state/ducks/items.ts | 59 ++++++++++++++- ts/state/selectors/items.ts | 18 +++++ ts/state/smart/ChatColorPicker.tsx | 19 +++-- ts/state/smart/GlobalModalContainer.tsx | 9 ++- .../state/ducks/conversations_test.ts | 63 ++++------------ 14 files changed, 250 insertions(+), 122 deletions(-) create mode 100644 ts/shims/reloadSelectedConversation.ts diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 717ffc2a5a..b82a6b8998 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -5454,10 +5454,18 @@ "message": "Reset chat color", "description": "Button label for resetting chat colors" }, + "ChatColorPicker__resetDefault": { + "message": "Reset Chat Colors", + "description": "Confirmation dialog title for resetting all chat colors or only the global default one" + }, "ChatColorPicker__resetAll": { "message": "Reset all chat colors", "description": "Button label for resetting all chat colors" }, + "ChatColorPicker__confirm-reset-default": { + "message": "Reset default", + "description": "Button label for resetting only global chat color" + }, "ChatColorPicker__confirm-reset": { "message": "Reset", "description": "Confirm button label for resetting chat colors" diff --git a/ts/background.ts b/ts/background.ts index 8df1ffe89b..c2a8242fd5 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -412,6 +412,12 @@ export async function startApp(): Promise { } first = false; + if (!window.storage.get('defaultConversationColor')) { + window.storage.put('defaultConversationColor', { + color: 'ultramarine', + }); + } + cleanupSessionResets(); const retryPlaceholders = new window.Signal.Util.RetryPlaceholders(); window.Signal.Services.retryPlaceholders = retryPlaceholders; diff --git a/ts/components/ChatColorPicker.stories.tsx b/ts/components/ChatColorPicker.stories.tsx index fd65cceb01..c51fc7e79b 100644 --- a/ts/components/ChatColorPicker.stories.tsx +++ b/ts/components/ChatColorPicker.stories.tsx @@ -32,6 +32,7 @@ const createProps = (): PropsType => ({ removeCustomColor: action('removeCustomColor'), removeCustomColorOnConversations: action('removeCustomColorOnConversations'), resetAllChatColors: action('resetAllChatColors'), + resetDefaultChatColor: action('resetDefaultChatColor'), selectedColor: select('selectedColor', ConversationColors, 'basil' as const), selectedCustomColor: {}, }); diff --git a/ts/components/ChatColorPicker.tsx b/ts/components/ChatColorPicker.tsx index 2ac5cd451c..ef22555782 100644 --- a/ts/components/ChatColorPicker.tsx +++ b/ts/components/ChatColorPicker.tsx @@ -27,7 +27,7 @@ export type PropsDataType = { customColors?: Record; getConversationsWithCustomColor: (colorId: string) => Array; i18n: LocalizerType; - isInModal?: boolean; + isGlobal?: boolean; onChatColorReset?: () => unknown; onSelectColor: ( color: ConversationColorType, @@ -46,6 +46,7 @@ type PropsActionType = { removeCustomColor: (colorId: string) => unknown; removeCustomColorOnConversations: (colorId: string) => unknown; resetAllChatColors: () => unknown; + resetDefaultChatColor: () => unknown; }; export type PropsType = PropsDataType & PropsActionType; @@ -56,16 +57,18 @@ export const ChatColorPicker = ({ editCustomColor, getConversationsWithCustomColor, i18n, - isInModal = false, + isGlobal = false, onChatColorReset, onSelectColor, removeCustomColor, removeCustomColorOnConversations, resetAllChatColors, + resetDefaultChatColor, selectedColor = ConversationColors[0], selectedCustomColor, }: PropsType): JSX.Element => { const [confirmResetAll, setConfirmResetAll] = useState(false); + const [confirmResetWhat, setConfirmResetWhat] = useState(false); const [customColorToEdit, setCustomColorToEdit] = useState< CustomColorDataType | undefined >(undefined); @@ -74,7 +77,7 @@ export const ChatColorPicker = ({ setCustomColorToEdit(undefined)} onSave={(color: CustomColorType) => { if (customColorToEdit?.id) { @@ -90,13 +93,39 @@ export const ChatColorPicker = ({ /> ); - if (isInModal && customColorToEdit) { + if (isGlobal && customColorToEdit) { return renderCustomColorEditorWrapper(); } return (
{customColorToEdit ? renderCustomColorEditorWrapper() : null} + {confirmResetWhat ? ( + { + resetDefaultChatColor(); + resetAllChatColors(); + }, + style: 'affirmative', + text: i18n('ChatColorPicker__resetAll'), + }, + ]} + i18n={i18n} + onClose={() => { + setConfirmResetWhat(false); + }} + title={i18n('ChatColorPicker__resetDefault')} + > + {i18n('ChatColorPicker__confirm-reset-message')} + + ) : null} {confirmResetAll ? ( { - setConfirmResetAll(true); + if (isGlobal) { + setConfirmResetWhat(true); + } else { + setConfirmResetAll(true); + } }} />
@@ -349,7 +382,7 @@ const CustomColorBubble = ({ type CustomColorEditorWrapperPropsType = { customColorToEdit?: CustomColorDataType; i18n: LocalizerType; - isInModal: boolean; + isGlobal: boolean; onClose: () => unknown; onSave: (color: CustomColorType) => unknown; }; @@ -357,7 +390,7 @@ type CustomColorEditorWrapperPropsType = { const CustomColorEditorWrapper = ({ customColorToEdit, i18n, - isInModal, + isGlobal, onClose, onSave, }: CustomColorEditorWrapperPropsType): JSX.Element => { @@ -370,7 +403,7 @@ const CustomColorEditorWrapper = ({ /> ); - if (!isInModal) { + if (!isGlobal) { return ( unknown; + setGlobalDefaultConversationColor: ( + color: ConversationColorType + ) => unknown; }) => JSX.Element; - setAllConversationColors: (color: ConversationColorType) => unknown; + setGlobalDefaultConversationColor: (color: ConversationColorType) => unknown; toggleChatColorEditor: () => unknown; }; @@ -20,7 +22,7 @@ export const GlobalModalContainer = ({ i18n, isChatColorEditorVisible, renderChatColorPicker, - setAllConversationColors, + setGlobalDefaultConversationColor, toggleChatColorEditor, }: PropsType): JSX.Element | null => { if (isChatColorEditorVisible) { @@ -34,7 +36,7 @@ export const GlobalModalContainer = ({ title={i18n('ChatColorPicker__global-chat-color')} > {renderChatColorPicker({ - setAllConversationColors, + setGlobalDefaultConversationColor, })} ); diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index a3dbb16a02..6a09532c69 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -24,6 +24,7 @@ import { AvatarColorType, AvatarColors, ConversationColorType, + CustomColorType, } from '../types/Colors'; import { MessageModel } from './messages'; import { isMuted } from '../util/isMuted'; @@ -1446,6 +1447,8 @@ export class ConversationModel extends window.Backbone .filter((member): member is ConversationType => member !== null) : undefined; + const { customColor, customColorId } = this.getCustomColorData(); + // TODO: DESKTOP-720 /* eslint-disable @typescript-eslint/no-non-null-assertion */ const result: ConversationType = { @@ -1469,8 +1472,8 @@ export class ConversationModel extends window.Backbone unblurredAvatarPath: this.getAbsoluteUnblurredAvatarPath(), color, conversationColor: this.getConversationColor(), - customColor: this.get('customColor'), - customColorId: this.get('customColorId'), + customColor, + customColorId, discoveredUnregisteredAt: this.get('discoveredUnregisteredAt'), draftBodyRanges, draftPreview, @@ -4871,8 +4874,33 @@ export class ConversationModel extends window.Backbone } getConversationColor(): ConversationColorType { - return (this.get('conversationColor') || - 'ultramarine') as ConversationColorType; + const defaultConversationColor = window.storage.get( + 'defaultConversationColor' + ); + + if (defaultConversationColor.customColorData) { + return 'custom'; + } + + return this.get('conversationColor') || defaultConversationColor.color; + } + + getCustomColorData(): { + customColor?: CustomColorType; + customColorId?: string; + } { + const defaultConversationColor = window.storage.get( + 'defaultConversationColor' + ); + + return { + customColor: + this.get('customColor') || + defaultConversationColor.customColorData?.value, + customColorId: + this.get('customColorId') || + defaultConversationColor.customColorData?.id, + }; } private getAvatarPath(): undefined | string { diff --git a/ts/models/messages.ts b/ts/models/messages.ts index 3201e4c608..d2e4a4d6c3 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -1002,7 +1002,7 @@ export class MessageModel extends window.Backbone.Model { textPending: this.get('bodyPending'), id: this.id, conversationColor: this.getConversationColor(), - customColor: conversation?.get('customColor'), + customColor: conversation?.getCustomColorData()?.customColor, conversationId: this.get('conversationId'), isSticker: Boolean(sticker), direction: this.isIncoming() ? 'incoming' : 'outgoing', diff --git a/ts/shims/reloadSelectedConversation.ts b/ts/shims/reloadSelectedConversation.ts new file mode 100644 index 0000000000..94019d9364 --- /dev/null +++ b/ts/shims/reloadSelectedConversation.ts @@ -0,0 +1,21 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +export function reloadSelectedConversation(): void { + const { conversations } = window.reduxStore.getState(); + const { selectedConversationId } = conversations; + if (!selectedConversationId) { + return; + } + const conversation = window.ConversationController.get( + selectedConversationId + ); + if (!conversation) { + return; + } + conversation.cachedProps = undefined; + window.reduxActions.conversations.conversationChanged( + conversation.id, + conversation.format() + ); +} diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index c736d90e84..5f88b3b2aa 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -379,7 +379,16 @@ type ColorsChangedActionType = { }; type CustomColorRemovedActionType = { type: typeof CUSTOM_COLOR_REMOVED; - payload: string; + payload: { + colorId: string; + defaultConversationColor: { + color: ConversationColorType; + customColorData?: { + id: string; + value: CustomColorType; + }; + }; + }; }; type SetPreJoinConversationActionType = { type: 'SET_PRE_JOIN_CONVERSATION'; @@ -697,7 +706,6 @@ export const actions = { reviewMessageRequestNameCollision, scrollToMessage, selectMessage, - setAllConversationColors, setComposeGroupAvatar, setComposeGroupName, setComposeSearchTerm, @@ -741,9 +749,16 @@ function removeCustomColorOnConversations( await window.Signal.Data.updateConversations(conversationsToUpdate); } + const defaultConversationColor = window.storage.get( + 'defaultConversationColor' + ); + dispatch({ type: CUSTOM_COLOR_REMOVED, - payload: colorId, + payload: { + colorId, + defaultConversationColor, + }, }); }; } @@ -769,44 +784,15 @@ function resetAllChatColors(): ThunkAction< delete conversation.attributes.customColorId; }); - dispatch({ - type: COLORS_CHANGED, - payload: { - conversationColor: undefined, - customColorData: undefined, - }, - }); - }; -} - -function setAllConversationColors( - conversationColor: ConversationColorType, - customColorData?: { - id: string; - value: CustomColorType; - } -): ThunkAction { - return async dispatch => { - await window.Signal.Data.updateAllConversationColors( - conversationColor, - customColorData + const defaultConversationColor = window.storage.get( + 'defaultConversationColor' ); - // We don't want to trigger a model change because we're updating redux - // here manually ourselves. Au revoir Backbone! - window.getConversations().forEach(conversation => { - Object.assign(conversation.attributes, { - conversationColor, - customColor: customColorData?.value, - customColorId: customColorData?.id, - }); - }); - dispatch({ type: COLORS_CHANGED, payload: { - conversationColor, - customColorData, + conversationColor: defaultConversationColor.color, + customColorData: defaultConversationColor.customColorData, }, }); }; @@ -2547,7 +2533,7 @@ export function reducer( if (action.type === CUSTOM_COLOR_REMOVED) { const { conversationLookup } = state; - const colorId = action.payload; + const { colorId, defaultConversationColor } = action.payload; const nextState = { ...state, @@ -2560,11 +2546,12 @@ export function reducer( return; } - const changed = omit(existing, [ - 'conversationColor', - 'customColor', - 'customColorId', - ]); + const changed = { + ...existing, + conversationColor: defaultConversationColor.color, + customColor: defaultConversationColor.customColorData?.value, + customColorId: defaultConversationColor.customColorData?.id, + }; Object.assign( nextState, diff --git a/ts/state/ducks/items.ts b/ts/state/ducks/items.ts index 010eaafc42..d0cedc050d 100644 --- a/ts/state/ducks/items.ts +++ b/ts/state/ducks/items.ts @@ -7,7 +7,12 @@ import { ThunkAction } from 'redux-thunk'; import { StateType as RootStateType } from '../reducer'; import * as storageShim from '../../shims/storage'; import { useBoundActions } from '../../util/hooks'; -import { CustomColorType } from '../../types/Colors'; +import { + ConversationColors, + ConversationColorType, + CustomColorType, +} from '../../types/Colors'; +import { reloadSelectedConversation } from '../../shims/reloadSelectedConversation'; // State @@ -15,6 +20,16 @@ export type ItemsStateType = { readonly universalExpireTimer?: number; readonly [key: string]: unknown; + + // This property should always be set and this is ensured in background.ts + readonly defaultConversationColor?: { + color: ConversationColorType; + customColorData?: { + id: string; + value: CustomColorType; + }; + }; + readonly customColors?: { readonly colors: Record; readonly version: number; @@ -63,6 +78,8 @@ export const actions = { addCustomColor, editCustomColor, removeCustomColor, + resetDefaultChatColor, + setGlobalDefaultConversationColor, onSetSkinTone, putItem, putItemExternal, @@ -184,10 +201,48 @@ function removeCustomColor( }; } +function resetDefaultChatColor(): ThunkAction< + void, + RootStateType, + unknown, + ItemPutAction +> { + return dispatch => { + dispatch( + putItem('defaultConversationColor', { + color: ConversationColors[0], + }) + ); + reloadSelectedConversation(); + }; +} + +function setGlobalDefaultConversationColor( + color: ConversationColorType, + customColorData?: { + id: string; + value: CustomColorType; + } +): ThunkAction { + return dispatch => { + dispatch( + putItem('defaultConversationColor', { + color, + customColorData, + }) + ); + reloadSelectedConversation(); + }; +} + // Reducer function getEmptyState(): ItemsStateType { - return {}; + return { + defaultConversationColor: { + color: ConversationColors[0], + }, + }; } export function reducer( diff --git a/ts/state/selectors/items.ts b/ts/state/selectors/items.ts index 65ac537704..43a26e3ff8 100644 --- a/ts/state/selectors/items.ts +++ b/ts/state/selectors/items.ts @@ -7,6 +7,11 @@ import { ITEM_NAME as UNIVERSAL_EXPIRE_TIMER_ITEM } from '../../util/universalEx import { StateType } from '../reducer'; import { ItemsStateType } from '../ducks/items'; +import { + ConversationColors, + ConversationColorType, + CustomColorType, +} from '../../types/Colors'; export const getItems = (state: StateType): ItemsStateType => state.items; @@ -25,3 +30,16 @@ export const getUniversalExpireTimer = createSelector( getItems, (state: ItemsStateType): number => state[UNIVERSAL_EXPIRE_TIMER_ITEM] || 0 ); + +export const getDefaultConversationColor = createSelector( + getItems, + ( + state: ItemsStateType + ): { + color: ConversationColorType; + customColorData?: { + id: string; + value: CustomColorType; + }; + } => state.defaultConversationColor ?? { color: ConversationColors[0] } +); diff --git a/ts/state/smart/ChatColorPicker.tsx b/ts/state/smart/ChatColorPicker.tsx index e60bbd2fbf..34ef2338b8 100644 --- a/ts/state/smart/ChatColorPicker.tsx +++ b/ts/state/smart/ChatColorPicker.tsx @@ -13,13 +13,13 @@ import { StateType } from '../reducer'; import { getConversationSelector, getConversationsWithCustomColorSelector, - getMe, } from '../selectors/conversations'; +import { getDefaultConversationColor } from '../selectors/items'; import { getIntl } from '../selectors/user'; export type SmartChatColorPickerProps = { conversationId?: string; - isInModal?: boolean; + isGlobal?: boolean; onChatColorReset?: () => unknown; onSelectColor: ( color: ConversationColorType, @@ -34,9 +34,14 @@ const mapStateToProps = ( state: StateType, props: SmartChatColorPickerProps ): PropsDataType => { - const conversation = props.conversationId + const defaultConversationColor = getDefaultConversationColor(state); + const colorValues = props.conversationId ? getConversationSelector(state)(props.conversationId) - : getMe(state); + : { + conversationColor: defaultConversationColor.color, + customColorId: defaultConversationColor.customColorData?.id, + customColor: defaultConversationColor.customColorData?.value, + }; const { customColors } = state.items; @@ -47,10 +52,10 @@ const mapStateToProps = ( state ), i18n: getIntl(state), - selectedColor: conversation.conversationColor, + selectedColor: colorValues.conversationColor, selectedCustomColor: { - id: conversation.customColorId, - value: conversation.customColor, + id: colorValues.customColorId, + value: colorValues.customColor, }, }; }; diff --git a/ts/state/smart/GlobalModalContainer.tsx b/ts/state/smart/GlobalModalContainer.tsx index e5876a7244..e1ac364a28 100644 --- a/ts/state/smart/GlobalModalContainer.tsx +++ b/ts/state/smart/GlobalModalContainer.tsx @@ -11,12 +11,15 @@ import { SmartChatColorPicker } from './ChatColorPicker'; import { ConversationColorType } from '../../types/Colors'; function renderChatColorPicker({ - setAllConversationColors, + setGlobalDefaultConversationColor, }: { - setAllConversationColors: (color: ConversationColorType) => unknown; + setGlobalDefaultConversationColor: (color: ConversationColorType) => unknown; }): JSX.Element { return ( - + ); } diff --git a/ts/test-electron/state/ducks/conversations_test.ts b/ts/test-electron/state/ducks/conversations_test.ts index 3fa54b6bde..0df0b0a2f2 100644 --- a/ts/test-electron/state/ducks/conversations_test.ts +++ b/ts/test-electron/state/ducks/conversations_test.ts @@ -41,7 +41,6 @@ const { openConversationInternal, repairNewestMessage, repairOldestMessage, - setAllConversationColors, setComposeGroupAvatar, setComposeGroupName, setComposeSearchTerm, @@ -2041,49 +2040,10 @@ describe('both/state/ducks/conversations', () => { }, }); - it('setAllConversationColors', async () => { - const dispatch = sinon.spy(); - await setAllConversationColors('crimson')(dispatch, getState, null); - - const [action] = dispatch.getCall(0).args; - const nextState = reducer(getState().conversations, action); - - sinon.assert.calledOnce(dispatch); - assert.equal( - nextState.conversationLookup.abc.conversationColor, - 'crimson' - ); - assert.equal( - nextState.conversationLookup.def.conversationColor, - 'crimson' - ); - assert.equal( - nextState.conversationLookup.ghi.conversationColor, - 'crimson' - ); - assert.equal( - nextState.conversationLookup.jkl.conversationColor, - 'crimson' - ); - assert.equal( - nextState.conversationsByUuid.abc.conversationColor, - 'crimson' - ); - assert.equal( - nextState.conversationsByUuid.def.conversationColor, - 'crimson' - ); - assert.equal( - nextState.conversationsByE164.ghi.conversationColor, - 'crimson' - ); - assert.equal( - nextState.conversationsByGroupId.jkl.conversationColor, - 'crimson' - ); - }); - it('resetAllChatColors', async () => { + window.storage.put('defaultConversationColor', { + color: 'crimson', + }); const dispatch = sinon.spy(); await resetAllChatColors()(dispatch, getState, null); @@ -2091,38 +2051,39 @@ describe('both/state/ducks/conversations', () => { const nextState = reducer(getState().conversations, action); sinon.assert.calledOnce(dispatch); - assert.isUndefined( + assert.equal( nextState.conversationLookup.abc.conversationColor, 'crimson' ); - assert.isUndefined( + assert.equal( nextState.conversationLookup.def.conversationColor, 'crimson' ); - assert.isUndefined( + assert.equal( nextState.conversationLookup.ghi.conversationColor, 'crimson' ); - assert.isUndefined( + assert.equal( nextState.conversationLookup.jkl.conversationColor, 'crimson' ); - assert.isUndefined( + assert.equal( nextState.conversationsByUuid.abc.conversationColor, 'crimson' ); - assert.isUndefined( + assert.equal( nextState.conversationsByUuid.def.conversationColor, 'crimson' ); - assert.isUndefined( + assert.equal( nextState.conversationsByE164.ghi.conversationColor, 'crimson' ); - assert.isUndefined( + assert.equal( nextState.conversationsByGroupId.jkl.conversationColor, 'crimson' ); + window.storage.remove('defaultConversationColor'); }); }); });