feat(TAC): add cross-room thread panels to Threads Activity Centre - #32851
feat(TAC): add cross-room thread panels to Threads Activity Centre#32851nathanael-h wants to merge 9 commits into
Conversation
|
Neat feature, I hope we get in in official version soon :) |
|
Looking nice, this would help me a lot in my workflows! |
|
I've used it for a few days, it was really useful. It would be nice if the display would be larger, though. The "Threads activity" panel is really small, even room names don't display entirely when they're more than a few characters long. |
a3a52ea to
7dbb512
Compare
|
I made some changes to make this modified Threads Activity Center more friendly, but I am almost certain that design team might have better suggestions!
|
|
Thanks for looking this PR @daniellekirkwood note that we're also an Element's customer. Feel free to ask me or us (Vates) directly. Following threads is a very very expected improvement expected here 😇 |
|
Hi there @nathanael-h Thank you very much for your contribution and proposal. Our product teams are currently looking into it and we will get back to you when we get an update. |
|
I just tested and and I realized how many threads I was missing 😆 IMHO, that's more than a cool addition, this is really important to make sure nobody misses a thread! Thanks @nathanael-h for that work! 👍 |
7dbb512 to
6d1bfa7
Compare
|
Hello @gaelledel and @daniellekirkwood I rebased this branch/PR on master yesterday. How can I help to move forward? I am very very open to hear feedback from Element design/product team! And follow advices, adapt this PR, add tests once we agree on implementation and expected UX. Our motivation is very high, as following conversation accross multiple spaces, sub-spaces, rooms and threads is not easy. And the risk is high to miss important messages. (Important in the context of a remote company where instant messaging is the first communication channel. [Side note, I'm looking also at everything related to https://github.com/matrix-org/matrix-spec-proposals/pull/4306] |
|
Hi @nathanael-h
Let me know if this is clear for you and if you'd like to talk it through more, we can arrange to get in touch. |
|
I wish "My threads" showed all threads I participated in, including read ones, as this is very useful for finding threads that I still need to respond to even if I've seen them. |
Including ideally threads where we reacted with an emoji to any message, which would be a convenient way to subscribe to a thread without having to write a meaningless message. |
|
@gaelledel Sorry I did not had time enough before this week to resume work on the PR. I rebased it, fixed some logic in threads list computation, fixed a navigation issue when opening a thread, and switched to more existing components. So now it is ready for a for round of design and code reviews. Regarding design I did not set with to 444, because I like TAC a little bit wider. But if it is a hard requirement, just say it, and I'll change it (or a dev can do it in my fork/ this PR). Regarding code review, I want to be fully transparent, I understand the changes done, but I heavily used claude code, and I'm not an expert like the maintainers of element-web. So two things, be careful when you merge this PR, and please be honest, I can improve with advises. I hope this PR can land sooner than later, because it help quite a bit in our company, and people have really a hard time to find where discussion happens. |
|
@nathanael-h Thanks a lot for the work! Do you have a netlify build so I can test out? We can also talk next week and review together. I will follow up with devs internally as well. |
|
@gaelledel I think the netlify automatic deployment needs an approval from one maintainer. There's element-pr32851.vates.tech/ but I've authorized only vates.tech accounts to login, and I'm AFK, so I couldn't modify it very soon. |
@florianduros Can you help unblock by any chance? |
|
@florianduros thanks, it is expected that playwright tests fail. When the PR would be a little bit more reviewed, I could work on this to provided updated screenshot references. |
|
@nathanael-h I have tested the impl. and it seems there is a logic issue with "my threads" tab? I don't see any content appearing here for threads I participate in - instead I see them in "other threads". Let me know when you return from hol so we can discuss |
|
Hello @gaelledel I have tested the netlify instance with my matrix.org account and did not had this issue, but it takes some times for the TAC to have threads data. This is probably unrelated to this PR as it is the same behavior as in app.element.io. I'm back if you wanna have a call next week, tell me. Screenshot of "My threads" with one unread thread:
Screenshot of "Other threads" with many unread threads. Note that I had to open the room at least once for the TAC to be populated and it took a few seconds (not instant) Same with TAC on app.element.io : |
|
Testing again and the issue I see is: I am Bob user > Alice created a thread from her Root msg > Bob adds a reply in Alice's thread |
Adds two new tabs to the TAC popup (Rooms / All threads / My threads): - "Rooms" tab: existing behaviour — rooms with unread threads - "All threads" tab: flat list of all unread threads from those rooms, shown as two-line rows (room avatar + name, sender + message preview) - "My threads" tab: same layout but filtered to threads the user participated in (as root sender or replier), across all rooms Implementation notes: - Export doesTimelineHaveUnreadMessages from Unread.ts for per-thread unread detection - Participation check covers both thread root and replies - Uses server-side notification counts as primary unread signal, falling back to timeline inspection for activity-level unread - settingTACOnlyNotifs applies to the Rooms tab only; thread tabs show all unread participated threads regardless of notification level Warning: I know threads and unread are very difficult topics. But in daily job we are many to strungle to find and follow threads, in which rooms and where. So I started this not to provide a perfect solution, but as starting point to sustain the improvment regarding this issue. (And to see if at least we could have something in-house) Honestly I think this would benefit a lot from the ongoing work on element-web new architecture - which I did not took time to dive in - and on every MSCs and synapse that will provide threads subscribe and unsubscribe mechanisms. Finally, this definetly needs design! Closes element-hq#28036
…chevron on hover Move the notification dot to the trailing edge of the thread row as a direct MenuItem child (order: 998). The compound nav-hint chevron sits at order: 999 in the same position. On hover, compound's `~ *` rule naturally hides the dot; a targeted override keeps the text content visible. This removes the hideChevron workaround entirely.
Replace 3 tabs (Rooms | All | My) with 2 mutually exclusive tabs: "My threads" (default) and "Other threads". Categorise via server data (thread.hasCurrentUserParticipated, highlight count); mentions/keywords land in My threads even when not participated. The tac_only_notifications setting is scoped to "Other threads" only: My threads always shows participated or highlighted threads regardless of the setting. In Other threads, setting=true drops activity-only (local-detected) entries; setting=false keeps them. Default flipped to false. The setting label now reads "In the Other threads tab, only show notifications". The rooms list and greatestNotificationLevel only reflect rooms that contribute at least one displayed thread. Drop the doesRoomHaveUnreadThreads() room-level pre-filter; it short-circuited on muted rooms and on rooms with no local timeline activity, hiding participated threads in muted rooms and missing server-pushed highlights that hadn't surfaced locally. The per-thread loop now decides inclusion based on server counts (authoritative) plus a lazy local fallback. Mute is applied per-room inside the Other threads branch only, preserving the "muted rooms don't clutter Other threads" guarantee while letting them still contribute to My threads. UI: Compound NavBar/NavItem for tabs, hide the menu title, fixed popup height for ~6 cells, 40px avatars. Zero Compound FloatingMenu's padding-block so the tabs + rows containers control their own spacing. Fix hover regression by overriding Compound's chevron-replaces-content sibling rule so the dot and content stay visible.
The local unread heuristic (doesTimelineHaveUnreadMessages) reports a participated thread as unread when the user replied but isn't the literal last sender — e.g. a later reaction/edit/redaction landed after their reply. The js-sdk's read shortcut only fires for the very last event (see the "second-last event" TODO in room-receipts.ts), so the latest incoming message, older than the user's reply, still reads as unread. Guard the local fallback with hasUnreadAfterMyLatestReply: treat the thread as read once the user's own latest reply is at/after the latest incoming message. Server notification counts still take precedence and the muted-room fallback is preserved.
Render thread-row previews via MessagePreviewStore (consistent edit/reply/ emote/HTML handling with the room list) and resolve sender names with getSenderName. Apply Compound Text typography to the room name, sender and preview, dropping the equivalent font rules from CSS. Open a thread with a two-card stack (thread list beneath the thread view) so the thread view gets a working back button, mirroring Action.ShowThread. Extract evaluateThreadUnread from computeUnreadThreadRooms and collapse the per-tab render branches into a single active-tab selection.
136d831 to
4d82b16
Compare
|
Thanks for the bug report @gaelledel I think it is now fixed by commit 11c06e7 |
| # won't switch pnpm versions mid-process, so bypass the version check instead of | ||
| # failing. Exported (not just a CLI flag) so it also applies to the nested | ||
| # `pnpm build` spawned by js-sdk's `prepare` lifecycle script. | ||
| export npm_config_pm_on_fail=ignore |
There was a problem hiding this comment.
Can we move to its own dedicated PR? Unrelated to this PR and is more about devx
There was a problem hiding this comment.
Done in #34906 sorry I forgot I had to use this script
| /** | ||
| * The two views available in the Threads Activity Centre popup. | ||
| */ | ||
| enum TACView { | ||
| MyThreads = "my_threads", | ||
| OtherThreads = "other_threads", | ||
| } |
There was a problem hiding this comment.
I'm in favour to use string union or const enum
| const [activeThreads, activeEmptyCaption] = | ||
| view === TACView.MyThreads | ||
| ? [ | ||
| roomsAndNotifications.participatingThreads, | ||
| _t("threads_activity_centre|no_participating_threads_unread"), | ||
| ] | ||
| : [roomsAndNotifications.otherThreads, _t("threads_activity_centre|no_other_unread_threads")]; |
There was a problem hiding this comment.
Can we move it to its own dedicated function to keep this component easy to read
| participatingThreads: Array<ThreadData>; | ||
| otherThreads: Array<ThreadData>; |
There was a problem hiding this comment.
nit
| participatingThreads: Array<ThreadData>; | |
| otherThreads: Array<ThreadData>; | |
| participatingThreads: ThreadData[]; | |
| otherThreads: ThreadData[]; |
| export type ThreadData = { | ||
| thread: Thread; | ||
| room: Room; | ||
| notificationLevel: NotificationLevel; | ||
| }; |
There was a problem hiding this comment.
Done, tell me if it's okay or need to be changed, I'm not used to your habits
| font-size: 13px; | ||
| padding: 12px 16px; | ||
| font: var(--cpd-font-body-sm-regular); | ||
| color: $secondary-content; |
| padding-right: 16px; | ||
| font-size: 13px; | ||
| padding: 12px 16px; | ||
| font: var(--cpd-font-body-sm-regular); |
There was a problem hiding this comment.
Use compound Text component or apply the correct letter spacing (cf https://github.com/element-hq/element-web/blob/develop/code_style.md#react rules 21-22)
| show_room_tile: true, | ||
| room_id: room.roomId, | ||
| metricsTrigger: "WebThreadsActivityCentre", | ||
| focusNext: "threadsPanel", |
There was a problem hiding this comment.
We don't need to open the thread panel ?
| /** | ||
| * Compute the greatest notification level of all thread, the list of rooms with unread threads, and their notification level. | ||
| * Compute the list of unread threads, split into "my threads" (relevant to the user) | ||
| * and "other threads" (everything else), along with notification levels. | ||
| * | ||
| * Categorisation (mutually exclusive): | ||
| * - "My threads": {@link Thread.hasCurrentUserParticipated} (from the server's | ||
| * `current_user_participated` field in bundled `m.thread` relations) OR the current | ||
| * user has sent a message in the thread's local timeline (catches replies the server | ||
| * bundle hasn't caught up with yet, see {@link hasCurrentUserSentInThread}) OR the | ||
| * thread has a server highlight count > 0 (a mention/keyword for the user). | ||
| * - "Other threads": every other unread thread. | ||
| * | ||
| * The `settingTACOnlyNotifs` setting (`Notifications.tac_only_notifications`) is | ||
| * **scoped to "Other threads" only**. Threads relevant to the user are always shown, | ||
| * regardless of the setting: | ||
| * | ||
| * - "My threads": always includes any unread thread the user has participated in | ||
| * (or has a highlight for), whether the unread comes from server notification | ||
| * counts or local timeline inspection. Muted rooms still contribute here — a | ||
| * thread you replied in, or where you were mentioned, should reach you even | ||
| * when the room itself is muted. | ||
| * - "Other threads": | ||
| * - setting = false (default): include both server-notified and local-activity | ||
| * unreads (but skip muted rooms — non-relevant threads from muted rooms are | ||
| * noise by definition). | ||
| * - setting = true: only include threads with server-reported counts (drops | ||
| * activity-only threads that the homeserver hasn't pushed notifications for). | ||
| * | ||
| * Local unread detection has known limitations (timeline window may not cover the | ||
| * full history); the setting lets users mute the noisier "Other threads" list while | ||
| * keeping personally-relevant threads visible. | ||
| * | ||
| * The `rooms` array and `greatestNotificationLevel` only reflect rooms that | ||
| * contribute at least one displayed thread, so the indicator badge matches what | ||
| * the user will actually see in the popup. | ||
| * | ||
| * Note: we intentionally do NOT pre-filter rooms via `doesRoomHaveUnreadThreads()`. | ||
| * That helper short-circuits on muted rooms and on rooms where the local timeline | ||
| * has no detected unread — both of which can mask server-flagged highlights and | ||
| * participated threads. Iterating per-thread is cheap (server counts are O(1) | ||
| * lookups) and avoids those false negatives. |
There was a problem hiding this comment.
This documentation is going necessary into details, can we keep it simpler?
| * counts and falling back to local timeline inspection for threads the server hasn't | ||
| * pushed counts for. | ||
| * | ||
| * The local fallback is needed because {@link doesTimelineHaveUnreadMessages} can report a |
There was a problem hiding this comment.
This seems an issue in doesTimelineHaveUnreadMessages. Imo instead doing some workaround here, we should fix doesTimelineHaveUnreadMessages instead. I suggest to keep using doesTimelineHaveUnreadMessages and open an issue for incorrect behaviour so we can fix it later.
There was a problem hiding this comment.
Done in matrix-org/matrix-js-sdk#5516 workaround kept until PR get merged (because I run an instance daily from this branch, that could changed if we want to fast track this TAC improvement and address the 2 PRs with fixes later)
Thread.hasCurrentUserParticipated is set only from the homeserver's bundled current_user_participated flag, which lags behind local activity. Right after the user replies in someone else's thread, the flag is still false, so the thread was wrongly categorised under Other threads. Complement the server flag by inspecting the local timeline: if the current user authored a sent m.thread reply, the thread is theirs. This matches the server's participated semantics (isRelation(m.thread) && !status), the same test the rest of the app uses for a real reply.
Upstream migrated the Threads Activity Centre unit tests from jest (test/unit-tests/.../*-test.tsx) to colocated vitest (src/.../*.test.tsx) in element-hq#34800. Re-apply our redesigned My threads / Other threads test suite on top of that migration: - Convert jest.* -> vi.* and jest-matrix-react -> test-utils-rtl imports. - Replace the pre-redesign test bodies with the participated/other-threads suite matching the current component API. - Update notification-badge assertions to the shared NotificationBadgeView data-notification-level attribute (NotificationBadge MVVM refactor). - Drop the obsolete ThreadsActivityCentre snapshot (redesign uses explicit assertions, no snapshots).
- replace `enum TACView` with a string union - extract tab selection into `getActiveTabContent` - `Array<ThreadData>` -> `ThreadData[]`, `Result` -> exported `UnreadThreadRooms` - add tsdoc to `ThreadData` - trim `computeUnreadThreadRooms` tsdoc - restore `tac_only_notifications` default to true - use Compound spacing tokens, incl. `--cpd-space-0-5x` for the 2px gap - use Compound semantic colour tokens instead of `$*-content` - use the Compound `Text` component for the empty caption - drop all `!important` added by this PR, scoping rules under `[role="menu"]` instead - restore `hideChevron` on `MenuItem` rather than hiding the chevron in CSS - drop the dead `mx_ThreadsActivityCentreRow` class - link the upstream issues/PRs from both workaround tsdocs
4d82b16 to
e7b51e9
Compare
|
Hello @florianduros thanks for the review, I pushed the requested fixes. I also opened other PRs. Until they get merged, I kept the local workarounds here. Tell me if you want to fast track this PR and have it merged before the newer PR. And if that's what you want, I'd remove the workaround here. |







EDIT: PR moved since initial publish, look at latest messages for latest status.
Adds two new tabs to the TAC popup (Rooms / All threads / My threads):
Implementation notes:
Warning:
I know threads and unread are very difficult topics. But in daily job we are many to strungle to find and follow threads, in which rooms and where. So I started this not to provide a perfect solution, but as starting point to sustain the improvment regarding this issue. (And to see if at least we could have something in-house) Honestly I think this would benefit a lot from the ongoing work on element-web new architecture - which I did not took time to dive in - and on every MSCs and synapse that will provide threads subscribe and unsubscribe mechanisms.
Finally, this definitely needs design!
Closes #28036
Checklist
public/exportedsymbols have accurate TSDoc documentation.