From a26d8ef61f6aec99348b2a595c31e41391898c50 Mon Sep 17 00:00:00 2001 From: Ryan Miller <ryan.m@getsession.org> Date: Wed, 14 Aug 2024 18:06:37 +1000 Subject: [PATCH 1/4] feat: create custom tag parser and tag renderer --- ts/components/basic/I18n.tsx | 96 ++++++++++++++----- .../basic/SessionCustomTagRenderer.tsx | 32 ++++--- ts/types/Localizer.ts | 50 +++++++++- 3 files changed, 140 insertions(+), 38 deletions(-) diff --git a/ts/components/basic/I18n.tsx b/ts/components/basic/I18n.tsx index 022ba1737..14f9109c5 100644 --- a/ts/components/basic/I18n.tsx +++ b/ts/components/basic/I18n.tsx @@ -1,4 +1,3 @@ -import React from 'react'; import type { GetMessageArgs, I18nProps, @@ -6,10 +5,16 @@ import type { LocalizerToken, } from '../../types/Localizer'; -import { useSelector } from 'react-redux'; +import { Fragment } from 'react'; import styled from 'styled-components'; -import { isDarkTheme } from '../../state/selectors/theme'; +import { useIsDarkTheme } from '../../state/selectors/theme'; import { SessionHtmlRenderer } from './SessionHTMLRenderer'; +import { + type CustomTag, + CustomTagProps, + SessionCustomTagRenderer, + supportedCustomTags, +} from './SessionCustomTagRenderer'; /** An array of supported html tags to render if found in a string */ const supportedFormattingTags = ['b', 'i', 'u', 's', 'br', 'span']; @@ -20,25 +25,22 @@ const formattingTagRegex = new RegExp( 'g' ); -const supportedCustomTags = ['emoji']; +const customTagRegex = new RegExp(`<(${supportedCustomTags.join('|')})/>`, 'g'); -const customTagRegex = new RegExp( - `<(?:${supportedFormattingTags.join('|')})>.*?</(?:${supportedCustomTags.join('|')})>`, - 'g' -); - -const StyledHtmlRenderer = styled.span<{ darkMode: boolean }>` +const StyledHtmlRenderer = styled.span<{ isDarkTheme: boolean }>` span { - color: ${props => (props.darkMode ? 'var(--primary-color)' : 'var(--text-primary-color)')}; + color: ${props => (props.isDarkTheme ? 'var(--primary-color)' : 'var(--text-primary-color)')}; } `; /** * Retrieve a localized message string, substituting dynamic parts where necessary and formatting it as HTML if necessary. * - * @param token - The token identifying the message to retrieve and an optional record of substitution variables and their replacement values. - * @param args - An optional record of substitution variables and their replacement values. This is required if the string has dynamic parts. - * @param as - An optional HTML tag to render the component as. Defaults to a fragment, unless the string contains html tags. In that case, it will render as HTML in a div tag. + * @param props.token - The token identifying the message to retrieve and an optional record of substitution variables and their replacement values. + * @param props.args - An optional record of substitution variables and their replacement values. This is required if the string has dynamic parts. + * @param props.as - An optional HTML tag to render the component as. Defaults to a fragment, unless the string contains html tags. In that case, it will render as HTML in a div tag. + * @param props.startTagProps - An optional object of props to pass to the start tag. + * @param props.endTagProps - An optional object of props to pass to the end tag. * * @returns The localized message string with substitutions and formatting applied. * @@ -50,7 +52,8 @@ const StyledHtmlRenderer = styled.span<{ darkMode: boolean }>` * ``` */ export const I18n = <T extends LocalizerToken>(props: I18nProps<T>) => { - const darkMode = useSelector(isDarkTheme); + const isDarkTheme = useIsDarkTheme(); + const i18nArgs = 'args' in props ? props.args : undefined; const i18nString = window.i18n<T, LocalizerDictionary[T]>( @@ -58,18 +61,59 @@ export const I18n = <T extends LocalizerToken>(props: I18nProps<T>) => { ); const containsFormattingTag = i18nString.match(formattingTagRegex); - const containsCustomTag = i18nString.match(customTagRegex); - /** If the string contains a relevant formatting tag, render it as HTML */ - if (containsFormattingTag || containsCustomTag) { - return ( - <StyledHtmlRenderer darkMode={darkMode}> - <SessionHtmlRenderer tag={props.as} html={i18nString} /> - </StyledHtmlRenderer> - ); - } + let startTag: CustomTag | null = null; + let endTag: CustomTag | null = null; - const Comp = props.as ?? React.Fragment; + /** + * @param match - The entire match, including the custom tag. + * @param group - The custom tag, without the angle brackets. + * @param index - The index of the match in the string. + */ + i18nString.replace(customTagRegex, (match: string, group: CustomTag, index: number) => { + if (index === 0) { + startTag = group; + } else if (index === i18nString.length - match.length) { + endTag = group; + } else { + /** + * If the match is not at the start or end of the string, throw an error. + * NOTE: This should never happen as this rule is enforced when the dictionary is generated. + */ + throw new Error( + `Custom tag ${group} (${match}) is not at the start or end (i=${index}) of the string: ${i18nString}` + ); + } - return <Comp>{i18nString}</Comp>; + return ''; + }); + + const content = containsFormattingTag ? ( + /** If the string contains a relevant formatting tag, render it as HTML */ + <StyledHtmlRenderer isDarkTheme={isDarkTheme}> + <SessionHtmlRenderer tag={props.as} html={i18nString} /> + </StyledHtmlRenderer> + ) : ( + i18nString + ); + + const Comp = props.as ?? Fragment; + + return ( + <Comp> + {startTag ? ( + <SessionCustomTagRenderer + tag={startTag} + tagProps={props.startTagProps as CustomTagProps<typeof startTag>} + /> + ) : null} + {content} + {endTag ? ( + <SessionCustomTagRenderer + tag={endTag} + tagProps={props.endTagProps as CustomTagProps<typeof endTag>} + /> + ) : null} + </Comp> + ); }; diff --git a/ts/components/basic/SessionCustomTagRenderer.tsx b/ts/components/basic/SessionCustomTagRenderer.tsx index 10704e06e..7ae662a6d 100644 --- a/ts/components/basic/SessionCustomTagRenderer.tsx +++ b/ts/components/basic/SessionCustomTagRenderer.tsx @@ -1,30 +1,40 @@ -import React from 'react'; -import { nativeEmojiData } from '../../util/emoji'; import styled from 'styled-components'; +import { nativeEmojiData } from '../../util/emoji'; const StyledEmoji = styled.span` font-size: 36px; margin-left: 8px; `; +export const supportedCustomTags = ['emoji'] as const; + +export type CustomTag = (typeof supportedCustomTags)[number]; + +/** + * A dictionary of custom tags and their rendering functions. + */ export const customTag = { emoji: ({ emoji }: { emoji: string }) => ( <StyledEmoji role={'img'} aria-label={nativeEmojiData?.ariaLabels?.[emoji]}> {emoji} </StyledEmoji> ), -}; +} as const; + +export type CustomTagProps<Tag extends CustomTag> = Parameters<(typeof customTag)[Tag]>[0]; -export const SessionCustomTag = <Tag extends keyof typeof customTag>({ +/** + * Render a custom tag with its props. + * + * @param tag - The custom tag to render. + * @param tagProps - The props to pass to the custom tag. + */ +export const SessionCustomTagRenderer = <Tag extends CustomTag>({ tag, - props, + tagProps, }: { tag: Tag; - props: Parameters<(typeof customTag)[Tag]>[0]; + tagProps: CustomTagProps<Tag>; }) => { - return customTag[tag](props); -}; - -export const SessionCustomTagRenderer = ({ str }: { str: string }) => { - const splitString = str.split(); + return customTag[tag](tagProps); }; diff --git a/ts/types/Localizer.ts b/ts/types/Localizer.ts index 66f96f667..12f0f9562 100644 --- a/ts/types/Localizer.ts +++ b/ts/types/Localizer.ts @@ -1,6 +1,7 @@ import type { ElementType } from 'react'; import type { Dictionary } from '../localization/locales'; import { LOCALE_DEFAULTS } from '../session/constants'; +import { CustomTag, CustomTagProps } from '../components/basic/SessionCustomTagRenderer'; /** A localization dictionary key */ type Token = keyof Dictionary; @@ -23,6 +24,7 @@ type DynamicArgs<LocalizedString extends string> = ? PluralVar | DynamicArgs<PluralOne> | DynamicArgs<PluralOther> : /** If a string segment has follows the variable form parse its variable name and recursively * check for more dynamic args */ + // eslint-disable-next-line @typescript-eslint/no-unused-vars -- We dont care about _Pre TODO: see if we can remove this infer LocalizedString extends `${infer _Pre}{${infer Var}}${infer Rest}` ? Var | DynamicArgs<Rest> : never; @@ -42,7 +44,13 @@ export type GetMessageArgs<T extends Token> = T extends Token : never; /** Basic props for all calls of the I18n component */ -type I18nBaseProps<T extends Token> = { token: T; as?: ElementType }; +type I18nBaseProps<T extends Token> = { + token: T; + as?: ElementType; + // TODO: investigate making these required when required and not required when not required + startTagProps?: CustomStartTagProps<T>; + endTagProps?: CustomEndTagProps<T>; +}; /** The props for the localization component */ export type I18nProps<T extends Token> = T extends Token @@ -53,6 +61,46 @@ export type I18nProps<T extends Token> = T extends Token : I18nBaseProps<T> & { args: ArgsRecordExcludingDefaults<T> } : never; +/** The props for custom tags at the start of an i18n strings */ +export type CustomStartTagProps<T extends Token> = T extends Token + ? Dictionary[T] extends `<${infer Tag}/>${string}` + ? Tag extends CustomTag + ? CustomTagProps<Tag> + : never + : never + : never; + +/** + * This is used to find the end tag. TypeScript navigates from outwards to inwards when doing magic + * with strings. This means we need a recursive type to find the actual end tag. + * + * @example For the string `{name} reacted with <emoji/>` + * The first iteration will find `Tag` as `emoji` because it grabs the first `<` and the last `/>` + * Because it doesn't contain a `<` it will return the Tag. + * + * @example For the string `You, {name} & <span>1 other</span> reacted with <emoji/>` + * The first iteration will find `Tag` as `span>1 other</span> reacted with <emoji` because it + * grabs the first `<` and the last `/>, so we then check if Tag contains a `<`: + * - If it doesn't then we have found it; + * - If it does then we need to run it through the same process again to search deeper. + */ +type CustomEndTag<LocalizedString extends string> = + LocalizedString extends `${string}<${infer Tag}/>${string}` ? FindCustomTag<Tag> : never; + +type FindCustomTag<S extends string> = S extends CustomTag + ? S + : S extends `${string}<${infer Tag}` + ? Tag extends CustomTag + ? Tag + : FindCustomTag<Tag> + : never; + +/** The props for custom tags at the end of an i18n strings */ +type CustomEndTagProps<T extends Token> = + CustomEndTag<Dictionary[T]> extends CustomTag + ? CustomTagProps<CustomEndTag<Dictionary[T]>> + : never; + /** The dictionary of localized strings */ export type LocalizerDictionary = Dictionary; From 3f272244d2e431260a53a9f8100f8ca11801d80b Mon Sep 17 00:00:00 2001 From: Ryan Miller <ryan.m@getsession.org> Date: Wed, 14 Aug 2024 18:07:12 +1000 Subject: [PATCH 2/4] fix: use I18n component in subtleNotifications --- ts/components/conversation/SubtleNotification.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ts/components/conversation/SubtleNotification.tsx b/ts/components/conversation/SubtleNotification.tsx index 4e10d73ed..65f6df824 100644 --- a/ts/components/conversation/SubtleNotification.tsx +++ b/ts/components/conversation/SubtleNotification.tsx @@ -98,7 +98,7 @@ export const NoMessageInConversation = () => { // TODOLATER use this selector across the whole application (left pane excluded) const name = useSelectedNicknameOrProfileNameOrShortenedPubkey(); - const messageText = useMemo(() => { + const content = useMemo(() => { if (isMe) { return <I18n token="noteToSelfEmpty" />; } @@ -120,9 +120,7 @@ export const NoMessageInConversation = () => { return ( <Container data-testid="empty-conversation-notification"> - <TextInner> - <SessionHtmlRenderer html={messageText} /> - </TextInner> + <TextInner>{content}</TextInner> </Container> ); }; From 530c4b9fc1c0e407e89129bd1fa7db187b39cda7 Mon Sep 17 00:00:00 2001 From: Ryan Miller <ryan.m@getsession.org> Date: Wed, 14 Aug 2024 18:07:53 +1000 Subject: [PATCH 3/4] feat: refactor reaction popup to use i18n component --- .../message/reactions/ReactionPopup.tsx | 128 +++++++----------- 1 file changed, 47 insertions(+), 81 deletions(-) diff --git a/ts/components/conversation/message/reactions/ReactionPopup.tsx b/ts/components/conversation/message/reactions/ReactionPopup.tsx index a67c12f08..90f9c1efb 100644 --- a/ts/components/conversation/message/reactions/ReactionPopup.tsx +++ b/ts/components/conversation/message/reactions/ReactionPopup.tsx @@ -1,11 +1,8 @@ -import { useEffect, useState, useMemo } from 'react'; -import { useSelector } from 'react-redux'; +import { useMemo } from 'react'; import styled from 'styled-components'; -import { Data } from '../../../../data/data'; import { findAndFormatContact } from '../../../../models/message'; import { PubKey } from '../../../../session/types/PubKey'; -import { useIsDarkTheme } from '../../../../state/selectors/theme'; -import { nativeEmojiData } from '../../../../util/emoji'; +import { I18n } from '../../../basic/I18n'; export type TipPosition = 'center' | 'left' | 'right'; @@ -54,22 +51,6 @@ export const StyledPopupContainer = styled.div<{ tooltipPosition: TipPosition }> } `; -const StyledEmoji = styled.span` - font-size: 36px; - margin-left: 8px; -`; - -const StyledContacts = styled.span` - word-break: break-all; - span { - word-break: keep-all; - } -`; - -const StyledOthers = styled.span<{ isDarkTheme: boolean }>` - color: ${props => (props.isDarkTheme ? 'var(--primary-color)' : 'var(--text-primary-color)')}; -`; - const generateContactsString = ( senders: Array<string> ): { contacts: Array<string>; numberOfReactors: number; hasMe: boolean } => { @@ -90,70 +71,59 @@ const generateContactsString = ( return { contacts, hasMe, numberOfReactors }; }; -const generateReactionString = ( +const getI18nComponent = ( isYou: boolean, contacts: Array<string>, - numberOfReactors: number + numberOfReactors: number, + emoji: string ) => { const name = contacts[0]; const other_name = contacts[1]; switch (numberOfReactors) { case 1: - return isYou - ? window.i18n('emojiReactsHoverYou') - : window.i18n('emojiReactsHoverName', { name }); + return isYou ? ( + <I18n token="emojiReactsHoverYouDesktop" endTagProps={{ emoji }} /> + ) : ( + <I18n token="emojiReactsHoverNameDesktop" args={{ name }} endTagProps={{ emoji }} /> + ); case 2: - return isYou - ? window.i18n('emojiReactsHoverYouName', { name }) - : window.i18n('emojiReactsHoverTwoName', { name, other_name }); + return isYou ? ( + <I18n token="emojiReactsHoverYouNameDesktop" args={{ name }} endTagProps={{ emoji }} /> + ) : ( + <I18n + token="emojiReactsHoverTwoNameDesktop" + args={{ name, other_name }} + endTagProps={{ emoji }} + /> + ); case 3: - return isYou - ? window.i18n('emojiReactsHoverYouNameOne', { name }) - : window.i18n('emojiReactsHoverTwoNameOne', { name, other_name }); + return isYou ? ( + <I18n token="emojiReactsHoverYouNameOneDesktop" args={{ name }} endTagProps={{ emoji }} /> + ) : ( + <I18n + token="emojiReactsHoverTwoNameOneDesktop" + args={{ name, other_name }} + endTagProps={{ emoji }} + /> + ); default: - return isYou - ? window.i18n('emojiReactsHoverYouNameMultiple', { - name, - count: numberOfReactors - 2, - }) - : window.i18n('emojiReactsHoverTwoNameMultiple', { - name, - other_name, - count: numberOfReactors - 2, - }); + return isYou ? ( + <I18n + token="emojiReactsHoverYouNameMultipleDesktop" + args={{ name, count: numberOfReactors - 2 }} + endTagProps={{ emoji }} + /> + ) : ( + <I18n + token="emojiReactsHoverTwoNameMultipleDesktop" + args={{ name, other_name, count: numberOfReactors - 2 }} + endTagProps={{ emoji }} + /> + ); } }; -const Contacts = (contacts: Array<string>, count: number) => { - const isDarkTheme = useIsDarkTheme(); - - const isYou = contacts[0] === window.i18n('you'); - const reactionPopupKey = useMemo( - () => reactionKey(isYou, contacts.length), - [isYou, contacts.length] - ); - - return ( - <StyledContacts> - {window.i18n(reactionPopupKey, { - name: contacts[0], - other_name: contacts[1], - count: contacts.length, - emoji: '', - })}{' '} - {contacts.length > 3 ? ( - <StyledOthers darkMode={darkMode}> - {window.i18n(contacts.length === 4 ? 'otherSingular' : 'otherPlural', { - number: `${count - 3}`, - })} - </StyledOthers> - ) : null} - <span>{window.i18n('reactionPopup')}</span> - </StyledContacts> - ); -}; - type Props = { messageId: string; emoji: string; @@ -163,26 +133,22 @@ type Props = { onClick: (...args: Array<any>) => void; }; -export const ReactionPopup = (props: Props): ReactElement => { - const { emoji, count, senders, tooltipPosition = 'center', onClick } = props; - const darkMode = useSelector(isDarkTheme); +export const ReactionPopup = (props: Props) => { + const { emoji, senders, tooltipPosition = 'center', onClick } = props; const { contacts, hasMe, numberOfReactors } = useMemo( () => generateContactsString(senders), [senders] ); - const reactionString = useMemo( - () => generateReactionString(hasMe, contacts, numberOfReactors), - [hasMe, contacts.length, numberOfReactors] + const content = useMemo( + () => getI18nComponent(hasMe, contacts, numberOfReactors, emoji), + [hasMe, contacts, numberOfReactors] ); return ( <StyledPopupContainer tooltipPosition={tooltipPosition} onClick={onClick}> - {contacts.length ? <StyledContacts>{Contacts(contacts, count)}</StyledContacts> : null} - <StyledEmoji role={'img'} aria-label={nativeEmojiData?.ariaLabels?.[emoji]}> - {emoji} - </StyledEmoji> + {content} </StyledPopupContainer> ); }; From 63369cf0084ce74f4bb4debaf104a0ae04746f0c Mon Sep 17 00:00:00 2001 From: Ryan Miller <ryan.m@getsession.org> Date: Wed, 14 Aug 2024 18:09:13 +1000 Subject: [PATCH 4/4] fix: getMessage default substitutions --- ts/util/i18n.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/ts/util/i18n.ts b/ts/util/i18n.ts index 76927ac91..c51e1efe4 100644 --- a/ts/util/i18n.ts +++ b/ts/util/i18n.ts @@ -94,8 +94,6 @@ export type Locale = keyof typeof timeLocaleMap; const enPluralFormRegex = /\{(\w+), plural, one \{(\w+)\} other \{(\w+)\}\}/; -const cardinalPluralFormRegex = /(zero|one|two|few|many|other) \{([^}]*)\}/g; - const cardinalPluralRegex: Record<Intl.LDMLPluralRule, RegExp> = { zero: /(zero) \{([^}]*)\}/, one: /(one) \{([^}]*)\}/, @@ -107,7 +105,7 @@ const cardinalPluralRegex: Record<Intl.LDMLPluralRule, RegExp> = { function getPluralKey(string: PluralString): PluralKey | undefined { const match = string.match(enPluralFormRegex); - return match ? match[1] : undefined; + return match && match[1] ? match[1] : undefined; } function getStringForCardinalRule( @@ -200,8 +198,10 @@ export const setupi18n = (locale: Locale, dictionary: LocalizerDictionary) => { return token as R; } - /** If a localized string does not have any arguments to substitute it is returned with no changes */ - if (!args) { + /** If a localized string does not have any arguments to substitute it is returned with no + * changes. We also need to check if the string contains a curly bracket as if it does + * there might be a default arg */ + if (!args && !localizedString.includes('{')) { return localizedString; } @@ -213,7 +213,7 @@ export const setupi18n = (locale: Locale, dictionary: LocalizerDictionary) => { `i18n: Attempted to nonexistent pluralKey for plural form string '${localizedString}'` ); } else { - const num = args[pluralKey] ?? 0; + const num = args?.[pluralKey] ?? 0; const cardinalRule = new Intl.PluralRules(locale).select(num); @@ -231,8 +231,9 @@ export const setupi18n = (locale: Locale, dictionary: LocalizerDictionary) => { } /** Find and replace the dynamic variables in a localized string and substitute the variables with the provided values */ + // @ts-expect-error TODO: Fix this type, now that we have plurals it doesnt quite work return localizedString.replace(/\{(\w+)\}/g, (match, arg: keyof typeof args) => { - const substitution = args[arg]; + const substitution: string | undefined = args?.[arg]; if (isUndefined(substitution)) { const defaultSubstitution = LOCALE_DEFAULTS[arg as keyof typeof LOCALE_DEFAULTS]; @@ -240,7 +241,8 @@ export const setupi18n = (locale: Locale, dictionary: LocalizerDictionary) => { return isUndefined(defaultSubstitution) ? match : defaultSubstitution; } - return substitution.toString(); + // TODO: figure out why is was type never and fix the type + return (substitution as string).toString(); }) as R; } catch (error) { i18nLog(`i18n: ${error.message}`);