From 1eec48e7d3b65c9e59412e81b23313fd30ba3980 Mon Sep 17 00:00:00 2001 From: Justin Date: Tue, 24 May 2022 16:30:47 -0400 Subject: [PATCH 1/2] Refactor notifications --- .../notifications/components/notification.tsx | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/app/soapbox/features/notifications/components/notification.tsx b/app/soapbox/features/notifications/components/notification.tsx index 9dd8a8bd0..b3453dde1 100644 --- a/app/soapbox/features/notifications/components/notification.tsx +++ b/app/soapbox/features/notifications/components/notification.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { HotKeys } from 'react-hotkeys'; -import { FormattedMessage, useIntl } from 'react-intl'; +import { defineMessages, IntlShape, MessageDescriptor } from 'react-intl'; +import { useIntl } from 'react-intl'; import { useHistory } from 'react-router-dom'; import { useAppSelector } from 'soapbox/hooks'; @@ -24,11 +25,6 @@ const notificationForScreenReader = (intl: ReturnType, message: return output.join(', '); }; -// Workaround for dynamic messages (https://github.com/formatjs/babel-plugin-react-intl/issues/119#issuecomment-326202499) -function FormattedMessageFixed(props: any) { - return ; -} - const buildLink = (account: Account): JSX.Element => ( = { user_approved: require('@tabler/icons/icons/user-plus.svg'), }; -const messages: Record = { +const messages: Record = defineMessages({ follow: { id: 'notification.follow', defaultMessage: '{name} followed you', @@ -100,18 +96,22 @@ const messages: Record id: 'notification.user_approved', defaultMessage: 'Welcome to {instance}!', }, -}; +}); -const buildMessage = (type: NotificationType, account: Account, targetName: string, instanceTitle: string): JSX.Element => { +const buildMessage = ( + intl: IntlShape, + type: NotificationType, + account: Account, + targetName: string, + instanceTitle: string, +): React.ReactNode => { const link = buildLink(account); - return ( - - ); + return intl.formatMessage(messages[type], { + name: link, + targetName, + instance: instanceTitle, + }); }; interface INotificaton { @@ -268,7 +268,7 @@ const Notification: React.FC = (props) => { const targetName = notification.target && typeof notification.target === 'object' ? notification.target.acct : ''; - const message: React.ReactNode = type && account && typeof account === 'object' ? buildMessage(type, account, targetName, instance.title) : null; + const message: React.ReactNode = type && account && typeof account === 'object' ? buildMessage(intl, type, account, targetName, instance.title) : null; return ( From 517c21ae52e2eb8de6d3a4b8c5ffb0ff83f891d0 Mon Sep 17 00:00:00 2001 From: Justin Date: Tue, 24 May 2022 16:49:59 -0400 Subject: [PATCH 2/2] Add grouped notifications messaging --- .../__tests__/notification.test.tsx | 25 +++++++++++++++++++ .../notifications/components/notification.tsx | 21 +++++++++++++--- app/soapbox/normalizers/notification.ts | 1 + .../reducers/__tests__/notifications-test.js | 6 +++++ 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/app/soapbox/features/notifications/components/__tests__/notification.test.tsx b/app/soapbox/features/notifications/components/__tests__/notification.test.tsx index f11d43f09..14c43f50c 100644 --- a/app/soapbox/features/notifications/components/__tests__/notification.test.tsx +++ b/app/soapbox/features/notifications/components/__tests__/notification.test.tsx @@ -28,6 +28,31 @@ describe('', () => { expect(screen.getByTestId('notification')).toBeInTheDocument(); expect(screen.getByTestId('account')).toContainHTML('neko@rdrama.cc'); + expect(screen.getByTestId('message')).toHaveTextContent('Nekobit followed you'); + }); + + describe('grouped notifications', () => { + it('renders a grouped follow notification for more than 2', async() => { + const { notification, state } = normalize(require('soapbox/__fixtures__/notification-follow.json')); + const groupedNotification = { ...notification.toJS(), total_count: 5 }; + + render(, undefined, state); + + expect(screen.getByTestId('notification')).toBeInTheDocument(); + expect(screen.getByTestId('account')).toContainHTML('neko@rdrama.cc'); + expect(screen.getByTestId('message')).toHaveTextContent('Nekobit + 4 others followed you'); + }); + + it('renders a grouped follow notification for 1', async() => { + const { notification, state } = normalize(require('soapbox/__fixtures__/notification-follow.json')); + const groupedNotification = { ...notification.toJS(), total_count: 2 }; + + render(, undefined, state); + + expect(screen.getByTestId('notification')).toBeInTheDocument(); + expect(screen.getByTestId('account')).toContainHTML('neko@rdrama.cc'); + expect(screen.getByTestId('message')).toHaveTextContent('Nekobit + 1 other followed you'); + }); }); it('renders a favourite notification', async() => { diff --git a/app/soapbox/features/notifications/components/notification.tsx b/app/soapbox/features/notifications/components/notification.tsx index b3453dde1..1a9dad8eb 100644 --- a/app/soapbox/features/notifications/components/notification.tsx +++ b/app/soapbox/features/notifications/components/notification.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { HotKeys } from 'react-hotkeys'; -import { defineMessages, IntlShape, MessageDescriptor } from 'react-intl'; +import { defineMessages, FormattedMessage, IntlShape, MessageDescriptor } from 'react-intl'; import { useIntl } from 'react-intl'; import { useHistory } from 'react-router-dom'; @@ -102,13 +102,27 @@ const buildMessage = ( intl: IntlShape, type: NotificationType, account: Account, + totalCount: number | null, targetName: string, instanceTitle: string, ): React.ReactNode => { const link = buildLink(account); + const name = intl.formatMessage({ + id: 'notification.name', + defaultMessage: '{link}{others}', + }, { + link, + others: totalCount && totalCount > 0 ? ( + + ) : '', + }); return intl.formatMessage(messages[type], { - name: link, + name, targetName, instance: instanceTitle, }); @@ -268,7 +282,7 @@ const Notification: React.FC = (props) => { const targetName = notification.target && typeof notification.target === 'object' ? notification.target.acct : ''; - const message: React.ReactNode = type && account && typeof account === 'object' ? buildMessage(intl, type, account, targetName, instance.title) : null; + const message: React.ReactNode = type && account && typeof account === 'object' ? buildMessage(intl, type, account, notification.total_count, targetName, instance.title) : null; return ( @@ -300,6 +314,7 @@ const Notification: React.FC = (props) => { theme='muted' size='sm' truncate + data-testid='message' > {message} diff --git a/app/soapbox/normalizers/notification.ts b/app/soapbox/normalizers/notification.ts index cee69f7ce..2bac3f2b3 100644 --- a/app/soapbox/normalizers/notification.ts +++ b/app/soapbox/normalizers/notification.ts @@ -34,6 +34,7 @@ export const NotificationRecord = ImmutableRecord({ status: null as EmbeddedEntity, target: null as EmbeddedEntity, // move type: '' as NotificationType | '', + total_count: null as number | null, // grouped notifications }); export const normalizeNotification = (notification: Record) => { diff --git a/app/soapbox/reducers/__tests__/notifications-test.js b/app/soapbox/reducers/__tests__/notifications-test.js index aa7d6f714..7061c60bb 100644 --- a/app/soapbox/reducers/__tests__/notifications-test.js +++ b/app/soapbox/reducers/__tests__/notifications-test.js @@ -274,6 +274,7 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: '😢', chat_message: null, + total_count: null, })], ['10743', ImmutableMap({ id: '10743', @@ -284,6 +285,7 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: null, chat_message: null, + total_count: null, })], ['10741', ImmutableMap({ id: '10741', @@ -294,6 +296,7 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: null, chat_message: null, + total_count: null, })], ['10734', ImmutableMap({ id: '10734', @@ -339,6 +342,7 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: '😢', chat_message: null, + total_count: null, })], ['10743', ImmutableMap({ id: '10743', @@ -349,6 +353,7 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: null, chat_message: null, + total_count: null, })], ['10741', ImmutableMap({ id: '10741', @@ -359,6 +364,7 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: null, chat_message: null, + total_count: null, })], ]), unread: 1,