Linear issue:
https://linear.app/comm/issue/ENG-1357/rename-sidebar-to-thread-in-product-copy
Diff renames sidebar to thread in all strings displayed to the user in web
Details
Details
Checked if strings changed as expected on web and/or mobile.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/shared/messages/create-sub-thread-message-spec.js | ||
|---|---|---|
| 128 ↗ | (On Diff #14522) | If we are looking at a thread(sidebar) does this function even get called? The function this appears in - createSubThreadMessageSpec is only referenced in lib/shared/message-specs.js where it is assigned to messageSpecs[messageTypes.CREATE_SUB_THREAD], whereas messageSpecs[messageTypes.CREATE_SIDEBAR] has createSidebarMessageSpec assigned to it. I don't believe this case makes sense here, nor did I manage to observe it in the app. |
| native/chat/chat-thread-list-see-more-sidebars.react.js | ||
| 29 ↗ | (On Diff #14522) | I can't seem to find a place where 'See threads....' would appear... having multiple threads(sidebars) in one chat causes 'See more...' to appear under the chat. Was it supposed to be there? |
| web/chat/chat-thread-list-see-more-sidebars.react.js | ||
| 25 ↗ | (On Diff #14522) | I can't seem to find a place where 'See threads....' would appear on web either |
| lib/shared/messages/create-sub-thread-message-spec.js | ||
|---|---|---|
| 128 ↗ | (On Diff #14522) | Looks like D598 was when this was introduced, but I haven't looked too closely. |
| native/chat/chat-thread-list-see-more-sidebars.react.js | ||
| 29 ↗ | (On Diff #14522) | Yeah, this can be confusing. I haven't looked thoroughly so I don't know the answer, but D536 is the diff that introduces showingSidebarsInline. Specifically, it looks like its defining logic is in chat-selectors.js. |
| native/chat/chat-thread-list-see-more-sidebars.react.js | ||
|---|---|---|
| 29 ↗ | (On Diff #14522) | Ohhh I see. Thank you! |
| lib/shared/messages/create-sub-thread-message-spec.js | ||
|---|---|---|
| 128 ↗ | (On Diff #14522) | First, some context: CREATE_SUB_THREAD is a message that appears in the parent when a child chat is created. In contrast, CREATE_THREAD and CREATE_SIDEBAR messages appear in the newly created chat (more details on those two here). Admittedly, this is rather confusing. We should probably add some code comments in the messageTypes definition to clarify this... probably best for a separate Linear task / diff, though. Regarding your question – I initially suspected you were right, that this case is not used. It doesn't look like a messageTypes.CREATE_SUB_THREAD will get created if the child is a sidebar. However, I then ran a query on my keyserver to confirm this: SELECT m.*, t.* FROM messages m LEFT JOIN threads t ON t.id = m.content WHERE m.type = 3 AND t.type = 5; It turns out that there is exactly one CREATE_SUB_THREAD that refers to a SIDEBAR, for some reason. This probably happened in the past when we were still iterating on the code for the sidebars feature. Perhaps this was the reason that D598 was created. Either way, it means we cannot remove this case here, I think. |
| native/chat/chat-thread-list-see-more-sidebars.react.js | ||
| 29 ↗ | (On Diff #14522) | After reading through this code I think you're right, we don't ever show See threads.... For everybody's context, this code refers to the inbox, which shows a list of chats. Underneath each "top-level" (not sidebar) chat, we show a list of sidebars. We don't show all sidebars, however... we have some special logic that decides which sidebars to show. I think in the past, if there were any sidebars at all (even if we didn't have any to show) we would still show ChatThreadListSeeMoreSidebars. However, at some point I think we decided to stop doing this. I think it was in D1487. Probably D1487 should have deleted showingSidebarsInline, but I forgot to do this / didn't realize I should. We should maybe delete it, but in a separate Linear task / diff. |
| web/modals/chat/sidebar-promote-modal.react.js | ||
| 35 ↗ | (On Diff #14522) | |
| lib/shared/messages/create-sub-thread-message-spec.js | ||
|---|---|---|
| 128 ↗ | (On Diff #14522) | Okay, I understand. I created a linear task to add these comments (assigned to me) |
| native/chat/chat-thread-list-see-more-sidebars.react.js | ||
| 29 ↗ | (On Diff #14522) | Should I create a linear task for this as well then? |
| web/modals/chat/sidebar-promote-modal.react.js | ||
| 35 ↗ | (On Diff #14522) | I added D4539 as a parent revision to this diff. I understand this is what I was supposed to do. Please correct me if I'm wrong. |
| lib/shared/messages/create-sub-thread-message-spec.js | ||
|---|---|---|
| 128 ↗ | (On Diff #14522) | |
| lib/shared/messages/create-sub-thread-message-spec.js | ||
|---|---|---|
| 128 ↗ | (On Diff #14522) | Thank you!! I realized you probably haven't read the Linear Guide, but one thing you should do when creating a task is to subscribe relevant people. In this case you can probably just subscribe people working on the JS codebase – me, @palys-swm, @atul, @def-au1t, and @yayabosh |
| native/chat/chat-thread-list-see-more-sidebars.react.js | ||
| 29 ↗ | (On Diff #14522) | Yes please! |
| web/modals/chat/sidebar-promote-modal.react.js | ||
| 35 ↗ | (On Diff #14522) | Yup, exactly! |