chat thread arrow layout is broken without breadcrumbs. If there isn't a parent thread, will just default to the thread name to not break the UI.
ios:
web:
https://linear.app/comm/issue/ENG-809/thread-arrows-are-not-aligned-correctly
Details
Details
web:
arrow shouldn't loose it position if there isn't a parnet element. You can patch the code and toggle the conditional to see that the arrows layout never breaks.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- fix-icon-paddings
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
this isn't ready.
Is it ready now? Saw you updated it a minute later but just wanted to make sure
Comment Actions
yes! I updated the diff to include something (the ui name) in the top line if it's the root conversation. I thought the conversation item would look off it was just empty space.
Comment Actions
Hmm, I'm not sure the logic here is right?
wrt the image you attached as being fixed:
I'm pretty sure we should have GENESIS above "ashoat"
Comment Actions
Hmm, I'm not sure the logic here is right?
wrt the image you attached as being fixed:
I'm pretty sure we should have GENESIS above "ashoat"
Sorry I should have added this to my description. Just updated it now:
The placeholder in the screenshot isn't the logic of the code. I didn't have a conversation in my local dev environment without a parent and just wanted to show what the placeholder would look like in practice.
Comment Actions
I didn't have a conversation in my local dev environment without a parent
I think the GENESIS thread?
Comment Actions
Production:
Yeah, it's weird. On development, the genesis thread will show on the top line of the chat thread item. But, not in production. (screenshot below). Assuming that something changed on the backend and genesis thread is now returned as a default.
Development:
Production:
Comment Actions
Why are we hacking this instead of fixing useAncestorThreads so native behaves in a consistent way here?
Comment Actions
@ashoat -- updated this diff and updated the useAncestorThreads hook so that the change is reflected in both native and web.
Comment Actions
Seems fine to me, but wondering if we should just return ancestorThreads directly. Curious to get @atul's perspective
| lib/shared/ancestor-threads.js | ||
|---|---|---|
| 17–22 ↗ | (On Diff #10160) | @atul did we ever conclude that we want to show the whole list here instead of excluding the current thread? I remember we had some discussion about this |
Comment Actions
@atul did we ever conclude that we want to show the whole list here instead of excluding the current thread? I remember we had some discussion about this
Yeah, at the time we decided to exclude the current thread from the path.
If I remember correctly:
- We concluded that including the thread name in the "path" was redundant and didn't provide any new information. We figured that "the point" of the path was to help users figure out "where" a given thread was, and including the current thread didn't provide any value.
- Visually, we didn't want the ancestor path to get overwhelmingly long at which point we "naively" cut off the end with ellipsis. We decided to re-assess this if we happened to do the "notion-style" interactive ellipsis in the thread list or if we decided to incorporate some sort of horizontal scrolling in the thread list (which we were both skeptical of).
- But basically decided to weigh minimizing visual clutter over marginal (if any) benefit to user.
However, we did decide that we wanted to include the entire path in the ThreadSettings component (where we have the interactive horizontal scrolling ancestors component) because:
- There was space for it
- It made sense visually since we were using arrows to show relationships between threads and it would be weird to exclude the one the user was in
- Clicking on the current thread in the ThreadSettings ancestry component navigates you to the thread, which keeps the navigation behavior symmetrical/predictable compared to the other threads listed