[MM-63023] Conditionally make thread item elements focusable based on whether they have an interaction (#30592)

* [MM-63023] Conditionally make thread item elements focusable based on whether they have an interaction * Fix i18n & PR feedback * Remove focusability from the text preview div * Fix tests
2025-04-22 15:55:30 -04:00
--- a/webapp/channels/src/components/threading/global_threads/thread_item/snapshots/thread_item.test.tsx.snap
+++ b/webapp/channels/src/components/threading/global_threads/thread_item/snapshots/thread_item.test.tsx.snap
@@ -1,10 +1,13 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

 exports[`components/threading/global_threads/thread_item should report total number of replies 1`] = `
-<article
+<div
+  aria-label="Thread by Someone"
  className="ThreadItem"
  id=""
  onClick={[Function]}
+  onKeyDown={[Function]}
+  role="link"
  tabIndex={0}
 >
  <header>
@@ -21,7 +24,7 @@ exports[`components/threading/global_threads/thread_item should report total num
        text="Team name"
      />
    </div>
-    <Connect(injectIntl(Timestamp))
+    <MockTimestamp
      className="alt-hidden"
      day="numeric"
      units={
@@ -39,7 +42,7 @@ exports[`components/threading/global_threads/thread_item should report total num
  <div
    className="menu-anchor alt-visible"
  >
-    <Memo(ThreadMenu)
+    <MockThreadMenu
      hasUnreads={false}
      isFollowing={true}
      threadId="1y8hpek81byspd4enyk9mp1ncw"
@@ -62,29 +65,22 @@ exports[`components/threading/global_threads/thread_item should report total num
          />
        </Memo(Button)>
      </WithTooltip>
-    </Memo(ThreadMenu)>
+    </MockThreadMenu>
  </div>
  <div
-    aria-readonly="true"
    className="preview"
    dir="auto"
    onClick={[Function]}
-    tabIndex={0}
+    onKeyDown={[Function]}
  >
-    <Connect(Markdown)
+    <MockMarkdown
      imageProps={
        Object {
          "onImageHeightChanged": [Function],
          "onImageLoaded": [Function],
        }
      }
-      mentionKeys={
-        Array [
-          Object {
-            "key": "@some-user",
-          },
-        ]
-      }
+      mentionKeys={Array []}
      message="test msg"
      options={
        Object {
@@ -98,7 +94,7 @@ exports[`components/threading/global_threads/thread_item should report total num
  <div
    className="activity"
  >
-    <Memo(Avatars)
+    <MockAvatars
      size="xs"
      userIds={
        Array [
@@ -118,14 +114,17 @@ exports[`components/threading/global_threads/thread_item should report total num
      }
    />
  </div>
-</article>
+</div>
 `;

 exports[`components/threading/global_threads/thread_item should report unread mentions 1`] = `
-<article
+<div
+  aria-label="Thread by Someone"
  className="ThreadItem has-unreads"
  id=""
  onClick={[Function]}
+  onKeyDown={[Function]}
+  role="link"
  tabIndex={0}
 >
  <header>
@@ -151,7 +150,7 @@ exports[`components/threading/global_threads/thread_item should report unread me
        text="Team name"
      />
    </div>
-    <Connect(injectIntl(Timestamp))
+    <MockTimestamp
      className="alt-hidden"
      day="numeric"
      units={
@@ -169,7 +168,7 @@ exports[`components/threading/global_threads/thread_item should report unread me
  <div
    className="menu-anchor alt-visible"
  >
-    <Memo(ThreadMenu)
+    <MockThreadMenu
      hasUnreads={true}
      isFollowing={true}
      threadId="1y8hpek81byspd4enyk9mp1ncw"
@@ -192,29 +191,22 @@ exports[`components/threading/global_threads/thread_item should report unread me
          />
        </Memo(Button)>
      </WithTooltip>
-    </Memo(ThreadMenu)>
+    </MockThreadMenu>
  </div>
  <div
-    aria-readonly="true"
    className="preview"
    dir="auto"
    onClick={[Function]}
-    tabIndex={0}
+    onKeyDown={[Function]}
  >
-    <Connect(Markdown)
+    <MockMarkdown
      imageProps={
        Object {
          "onImageHeightChanged": [Function],
          "onImageLoaded": [Function],
        }
      }
-      mentionKeys={
-        Array [
-          Object {
-            "key": "@some-user",
-          },
-        ]
-      }
+      mentionKeys={Array []}
      message="test msg"
      options={
        Object {
@@ -228,7 +220,7 @@ exports[`components/threading/global_threads/thread_item should report unread me
  <div
    className="activity"
  >
-    <Memo(Avatars)
+    <MockAvatars
      size="xs"
      userIds={
        Array [
@@ -248,14 +240,17 @@ exports[`components/threading/global_threads/thread_item should report unread me
      }
    />
  </div>
-</article>
+</div>
 `;

 exports[`components/threading/global_threads/thread_item should report unread messages 1`] = `
-<article
+<div
+  aria-label="Thread by Someone"
  className="ThreadItem has-unreads"
  id=""
  onClick={[Function]}
+  onKeyDown={[Function]}
+  role="link"
  tabIndex={0}
 >
  <header>
@@ -279,7 +274,7 @@ exports[`components/threading/global_threads/thread_item should report unread me
        text="Team name"
      />
    </div>
-    <Connect(injectIntl(Timestamp))
+    <MockTimestamp
      className="alt-hidden"
      day="numeric"
      units={
@@ -297,7 +292,7 @@ exports[`components/threading/global_threads/thread_item should report unread me
  <div
    className="menu-anchor alt-visible"
  >
-    <Memo(ThreadMenu)
+    <MockThreadMenu
      hasUnreads={true}
      isFollowing={true}
      threadId="1y8hpek81byspd4enyk9mp1ncw"
@@ -320,29 +315,22 @@ exports[`components/threading/global_threads/thread_item should report unread me
          />
        </Memo(Button)>
      </WithTooltip>
-    </Memo(ThreadMenu)>
+    </MockThreadMenu>
  </div>
  <div
-    aria-readonly="true"
    className="preview"
    dir="auto"
    onClick={[Function]}
-    tabIndex={0}
+    onKeyDown={[Function]}
  >
-    <Connect(Markdown)
+    <MockMarkdown
      imageProps={
        Object {
          "onImageHeightChanged": [Function],
          "onImageLoaded": [Function],
        }
      }
-      mentionKeys={
-        Array [
-          Object {
-            "key": "@some-user",
-          },
-        ]
-      }
+      mentionKeys={Array []}
      message="test msg"
      options={
        Object {
@@ -356,7 +344,7 @@ exports[`components/threading/global_threads/thread_item should report unread me
  <div
    className="activity"
  >
-    <Memo(Avatars)
+    <MockAvatars
      size="xs"
      userIds={
        Array [
@@ -376,5 +364,5 @@ exports[`components/threading/global_threads/thread_item should report unread me
      }
    />
  </div>
-</article>
+</div>
 `;
--- a/webapp/channels/src/components/threading/global_threads/thread_item/thread_item.test.tsx
+++ b/webapp/channels/src/components/threading/global_threads/thread_item/thread_item.test.tsx
@@ -27,6 +27,65 @@ jest.mock('mattermost-redux/actions/threads');

 jest.mock('actions/views/threads');

+jest.mock('utils/constants', () => ({
+    ...jest.requireActual('utils/constants'),
+    RelativeRanges: {
+        TODAY_TITLE_CASE: 'Today',
+        TOMORROW_TITLE_CASE: 'Tomorrow',
+        YESTERDAY_TITLE_CASE: 'Yesterday',
+        LAST_WEEK_TITLE_CASE: 'Last Week',
+        LAST_MONTH_TITLE_CASE: 'Last Month',
+        LAST_YEAR_TITLE_CASE: 'Last Year',
+    },
+    Integrations: {
+        EXECUTE_CURRENT_COMMAND_ITEM_ID: 'execute_current_command',
+        OPEN_COMMAND_IN_MODAL_ITEM_ID: 'open_command_in_modal',
+    },
+}));
+
+jest.mock('components/markdown', () => {
+    return function MockMarkdown({message}: {message: string}) {
+        if (message.includes('[link]')) {
+            return <a href='https://example.com'>{'link'}</a>;
+        }
+        return <span>{message}</span>;
+    };
+});
+
+jest.mock('components/post_markdown', () => ({
+    makeGetMentionKeysForPost: () => () => [],
+}));
+
+jest.mock('components/timestamp', () => {
+    return function MockTimestamp() {
+        return <span>{'timestamp'}</span>;
+    };
+});
+
+jest.mock('components/widgets/users/avatars', () => {
+    return function MockAvatars() {
+        return <div className='avatars'>{'avatars'}</div>;
+    };
+});
+
+jest.mock('./attachments', () => {
+    return function MockAttachment() {
+        return <div className='attachment'>{'attachment'}</div>;
+    };
+});
+
+jest.mock('components/tours/crt_tour/crt_list_tutorial_tip', () => {
+    return function MockCRTListTutorialTip() {
+        return <div className='tutorial-tip'>{'tutorial'}</div>;
+    };
+});
+
+jest.mock('../thread_menu', () => {
+    return function MockThreadMenu({children}: {children: React.ReactNode}) {
+        return <div className='thread-menu'>{children}</div>;
+    };
+});
+
 const mockRouting = {
    currentUserId: '7n4ach3i53bbmj84dfmu5b7c1c',
    currentTeamId: 'tid',
@@ -213,4 +272,24 @@ describe('components/threading/global_threads/thread_item', () => {
        expect(manuallyMarkThreadAsUnread).toHaveBeenCalledWith('1y8hpek81byspd4enyk9mp1ncw', 1611786714912);
        expect(mockDispatch).toHaveBeenCalledTimes(2);
    });
+
+    test('should set article tabIndex to -1 when thread is selected', () => {
+        const wrapper = shallow(
+            <ThreadItem
+                {...props}
+                isSelected={true}
+            />,
+        );
+        expect(wrapper.find('div.ThreadItem').prop('tabIndex')).toBe(-1);
+    });
+
+    test('should set article tabIndex to 0 when thread is not selected', () => {
+        const wrapper = shallow(
+            <ThreadItem
+                {...props}
+                isSelected={false}
+            />,
+        );
+        expect(wrapper.find('div.ThreadItem').prop('tabIndex')).toBe(0);
+    });
 });
--- a/webapp/channels/src/components/threading/global_threads/thread_item/thread_item.tsx
+++ b/webapp/channels/src/components/threading/global_threads/thread_item/thread_item.tsx
@@ -3,7 +3,7 @@

 import classNames from 'classnames';
 import React, {memo, useCallback, useEffect, useMemo} from 'react';
-import type {MouseEvent} from 'react';
+import type {MouseEvent, KeyboardEvent} from 'react';
 import {FormattedMessage, useIntl} from 'react-intl';
 import {useDispatch, useSelector} from 'react-redux';

@@ -119,7 +119,13 @@ function ThreadItem({

    let unreadTimestamp = post.edit_at || post.create_at;

-    const selectHandler = useCallback((e: MouseEvent<HTMLDivElement>) => {
+    const selectHandler = useCallback((e: MouseEvent<HTMLElement> | KeyboardEvent<HTMLElement>) => {
+        // If the event is a keyboard event, check if the key is 'Enter' or ' '.
+        if ('key' in e) {
+            if (e.key !== 'Enter' && e.key !== ' ') {
+                return;
+            }
+        }
        if (e.altKey) {
            const hasUnreads = thread ? Boolean(thread.unread_replies) : false;
            const lastViewedAt = hasUnreads ? Date.now() : unreadTimestamp;
@@ -153,6 +159,12 @@ function ThreadItem({
    }, [threadId]);

    const handleFormattedTextClick = useCallback((e) => {
+        // If the event is a keyboard event, check if the key is 'Enter' or ' '.
+        if ('key' in e) {
+            if (e.key !== 'Enter' && e.key !== ' ') {
+                return;
+            }
+        }
        Utils.handleFormattedTextClick(e, currentRelativeTeamUrl);
    }, [currentRelativeTeamUrl]);

@@ -176,15 +188,21 @@ function ThreadItem({
    }

    return (
-        <article
+        <div
            style={style}
            className={classNames('ThreadItem', {
                'has-unreads': newReplies,
                'is-selected': isSelected,
            })}
-            tabIndex={0}
+            tabIndex={isSelected ? -1 : 0}
+            role='link'
+            aria-label={formatMessage(
+                {id: 'threading.threadItem.ariaLabel', defaultMessage: 'Thread by {author}'},
+                {author: postAuthor},
+            )}
            id={isFirstThreadInList ? 'tutorial-threads-mobile-list' : ''}
            onClick={selectHandler}
+            onKeyDown={selectHandler}
        >
            <header>
                {Boolean(newMentions || newReplies) && (
@@ -246,12 +264,14 @@ function ThreadItem({
                    </WithTooltip>
                </ThreadMenu>
            </div>
+
+            {/* The strange interaction here where we need a click/keydown handler messes with the ESLint rules, so we just disable it */}
+            {/*eslint-disable-next-line jsx-a11y/no-static-element-interactions*/}
            <div
-                aria-readonly='true'
                className='preview'
                dir='auto'
-                tabIndex={0}
                onClick={handleFormattedTextClick}
+                onKeyDown={handleFormattedTextClick}
            >
                {post.message ? (
                    <Markdown
@@ -291,7 +311,7 @@ function ThreadItem({
                )}
            </div>
            {showListTutorialTip && isFirstThreadInList && isMobileView && (<CRTListTutorialTip/>)}
-        </article>
+        </div>
    );
 }

--- a/webapp/channels/src/i18n/en.json
+++ b/webapp/channels/src/i18n/en.json
@@ -5454,6 +5454,7 @@
  "threading.numNewReplies": "{newReplies, plural, =1 {# new reply} other {# new replies}}",
  "threading.numReplies": "{totalReplies, plural, =0 {Reply} =1 {# reply} other {# replies}}",
  "threading.threadHeader.menu": "More Actions",
+  "threading.threadItem.ariaLabel": "Thread by {author}",
  "threading.threadItem.menu": "Actions",
  "threading.threadList.markRead": "Mark all as read",
  "threading.threadMenu.copy": "Copy link",