From a10822bc7437b508cb0bc2f2bdb35adc65c7f710 Mon Sep 17 00:00:00 2001 From: William Grant Date: Wed, 17 May 2023 17:02:05 +1000 Subject: [PATCH] feat: improved robustness of changing the conversation header subtitle the previous logic relied on the length of the subtitles array which didn't account for when it changed depending on the conversation we were on --- .../header/ConversationHeaderSubtitle.tsx | 46 +++++++++--------- .../header/ConversationHeaderTitle.tsx | 48 +++++++++++++------ 2 files changed, 57 insertions(+), 37 deletions(-) diff --git a/ts/components/conversation/header/ConversationHeaderSubtitle.tsx b/ts/components/conversation/header/ConversationHeaderSubtitle.tsx index d44c4e92a..f8f7abc72 100644 --- a/ts/components/conversation/header/ConversationHeaderSubtitle.tsx +++ b/ts/components/conversation/header/ConversationHeaderSubtitle.tsx @@ -2,10 +2,10 @@ import React from 'react'; import styled, { CSSProperties } from 'styled-components'; import { Flex } from '../../basic/Flex'; import { SessionIconButton } from '../../icon'; +import { SubtitleStrings, SubtitleStringsType } from './ConversationHeaderTitle'; -// NOTE the subtitle toggling logic needs improvment -function loadDataTestId(subtitles: Array, currentIndex: number) { - if (currentIndex === subtitles.length - 1) { +function loadDataTestId(currentSubtitle: SubtitleStringsType) { + if (currentSubtitle === 'disappearingMessages') { return 'disappear-messages-type-and-time'; } else { return 'conversation-header-subtitle'; @@ -63,34 +63,36 @@ const SubtitleDotMenu = ({ ); type ConversationHeaderSubitleProps = { - subtitles: Array; - currentIndex: number; - setCurrentIndex: (index: number) => void; + subtitlesArray: Array; + subtitleStrings: SubtitleStrings; + currentSubtitle: SubtitleStringsType; + setCurrentSubtitle: (index: SubtitleStringsType) => void; onClickFunction: () => void; showDisappearingMessageIcon: boolean; }; export const ConversationHeaderSubitle = (props: ConversationHeaderSubitleProps) => { const { - subtitles, - currentIndex, - setCurrentIndex, + subtitlesArray, + subtitleStrings, + currentSubtitle, + setCurrentSubtitle, onClickFunction, showDisappearingMessageIcon, } = props; const handleTitleCycle = (direction: 1 | -1) => { - let newIndex = currentIndex + direction; - if (newIndex > subtitles.length - 1) { + let newIndex = subtitlesArray.indexOf(currentSubtitle) + direction; + if (newIndex > subtitlesArray.length - 1) { newIndex = 0; } if (newIndex < 0) { - newIndex = subtitles.length - 1; + newIndex = subtitlesArray.length - 1; } - if (subtitles[newIndex]) { - setCurrentIndex(newIndex); + if (subtitlesArray[newIndex]) { + setCurrentSubtitle(subtitlesArray[newIndex]); } }; @@ -99,7 +101,7 @@ export const ConversationHeaderSubitle = (props: ConversationHeaderSubitleProps) @@ -112,7 +114,7 @@ export const ConversationHeaderSubitle = (props: ConversationHeaderSubitleProps) onClick={() => { handleTitleCycle(-1); }} - isHidden={subtitles.length < 2} + isHidden={subtitlesArray.length < 2} tabIndex={0} /> {showDisappearingMessageIcon && ( @@ -134,9 +136,9 @@ export const ConversationHeaderSubitle = (props: ConversationHeaderSubitleProps) } }} tabIndex={0} - data-testid={loadDataTestId(subtitles, currentIndex)} + data-testid={loadDataTestId(currentSubtitle)} > - {subtitles[currentIndex]} + {subtitleStrings[currentSubtitle]} { handleTitleCycle(1); }} - isHidden={subtitles.length < 2} + isHidden={subtitlesArray.length < 2} tabIndex={0} /> ); diff --git a/ts/components/conversation/header/ConversationHeaderTitle.tsx b/ts/components/conversation/header/ConversationHeaderTitle.tsx index 6d374e0db..f856edcb9 100644 --- a/ts/components/conversation/header/ConversationHeaderTitle.tsx +++ b/ts/components/conversation/header/ConversationHeaderTitle.tsx @@ -15,6 +15,16 @@ import { } from '../../../util/expiringMessages'; import { ConversationHeaderSubitle } from './ConversationHeaderSubtitle'; +export type SubtitleStrings = Record & { + notifications?: string; + members?: string; + disappearingMessages?: string; +}; +export type SubtitleStringsType = keyof Pick< + SubtitleStrings, + 'notifications' | 'members' | 'disappearingMessages' +>; + export type ConversationHeaderTitleProps = { conversationKey: string; isMe: boolean; @@ -37,7 +47,7 @@ export const ConversationHeaderTitle = () => { const convoName = useConversationUsername(headerTitleProps?.conversationKey); const dispatch = useDispatch(); - const [visibleTitleIndex, setVisibleTitleIndex] = useState(0); + const [visibleSubtitle, setVisibleSubtitle] = useState('notifications'); if (!headerTitleProps) { return null; @@ -56,12 +66,15 @@ export const ConversationHeaderTitle = () => { const { i18n } = window; - const subtitles: Array = []; + const subtitleStrings: SubtitleStrings = {}; + const subtitleArray: Array = []; + const notificationSubtitle = notificationSetting ? i18n('notificationSubtitle', [notificationSetting]) : null; if (notificationSubtitle) { - subtitles.push(notificationSubtitle); + subtitleStrings.notifications = notificationSubtitle; + subtitleArray.push('notifications'); } let memberCount = 0; @@ -79,7 +92,8 @@ export const ConversationHeaderTitle = () => { memberCountSubtitle = isPublic ? i18n('activeMembers', [count]) : i18n('members', [count]); } if (memberCountSubtitle) { - subtitles.push(memberCountSubtitle); + subtitleStrings.members = memberCountSubtitle; + subtitleArray.push('members'); } const disappearingMessageSettingText = @@ -102,14 +116,15 @@ export const ConversationHeaderTitle = () => { }` : null; if (disappearingMessageSubtitle) { - subtitles.push(disappearingMessageSubtitle); + subtitleStrings.disappearingMessages = disappearingMessageSubtitle; + subtitleArray.push('disappearingMessages'); } const handleRightPanelToggle = () => { if (isRightPanelOn) { dispatch(closeRightPanel()); } else { - if (visibleTitleIndex === subtitles.length - 1) { + if (visibleSubtitle === 'disappearingMessages') { dispatch(setRightOverlayMode('disappearing-messages')); } else { dispatch(setRightOverlayMode('panel-settings')); @@ -119,14 +134,14 @@ export const ConversationHeaderTitle = () => { }; useEffect(() => { - setVisibleTitleIndex(0); + setVisibleSubtitle('notifications'); }, [convoName]); useEffect(() => { - if (!subtitles[visibleTitleIndex]) { - setVisibleTitleIndex(0); + if (subtitleArray.indexOf(visibleSubtitle) < 0) { + setVisibleSubtitle('notifications'); } - }, [visibleTitleIndex, subtitles]); + }, [subtitleArray, visibleSubtitle]); return (
@@ -150,13 +165,16 @@ export const ConversationHeaderTitle = () => { {convoName} )} - {subtitles && subtitles[visibleTitleIndex] && ( + {subtitleArray.length && subtitleArray.indexOf(visibleSubtitle) > -1 && ( )}