Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Threaded composer #3813

Closed
wants to merge 10 commits into from
Closed

Conversation

mary-ext
Copy link
Contributor

@mary-ext mary-ext commented May 2, 2024

image

image

Checklist

  • The actual threading UI
  • Decent-ish keyboard navigation (I think we can do better, perhaps by making use of Ctrl/Meta-Up/Down to control focus?)
  • External embeds
  • Tenor GIFs
  • Quote embeds
  • Image embeds
  • Post language
  • Gated reply interactions
  • Self-labeling
  • Send the post (demo)

Bonus checklist

  • ALF compliant composer

Problem

  1. This is the last component that makes use of MobX
    • Might be worth removing it here?
    • It doesn't seem worth for MobX removal to be in a separate PR, it'd potentially require double the work for old and new
  2. The language picker button is now even more problematic for smaller displays thanks to this addition
Archived
2024-05-02.13-15-46.mp4

@mary-ext mary-ext marked this pull request as draft May 2, 2024 06:21
@mary-ext
Copy link
Contributor Author

mary-ext commented May 2, 2024

gauging interest 😄 cc @haileyok @pfrazee

just need to know if this is something y'all want to pursue towards, because I have questions around the current image manipulation code and whether we could get rid of the last remaining MobX code on the app

@Spacedesolate
Copy link

Worst part of threads is having to use unroll bot, or clicking through replies to get to next commented reply, esp on those with 10+ posts.

So I think it would be great if threads are marked as such, instead of just posting as tweets and replies.

This way someone could post the entire tweet of 1000 words in one box, get to see how and where breakpoints would be applied, modify those if they wish.

And from readers side, one click option in 3 dot settings to unroll or view full post can make things lot easier.

@mary-ext
Copy link
Contributor Author

mary-ext commented May 2, 2024

@Spacedesolate that's pretty much out of scope for this work regardless if this makes it in or not (I'm just an outside contributor, I have no insight over Bluesky's roadmap or what the team intends to do)

consider making a new issue next time ^^

@pfrazee
Copy link
Collaborator

pfrazee commented May 2, 2024

Ooooh this is a very bold feature to pick up. We are interested in it. I'm quite worried that it's going to take some extensive testing and edgecase handling, and that we'll end up letting it languish because we're focusing on other things. My worst-case outcome is that you do a bunch of work that we don't honor. Let me talk to the crew tomorrow.

Regarding the mobx, that's just there because we were afraid to mess with the code. We do want to ditch it, it's just a finicky system.

@pfrazee
Copy link
Collaborator

pfrazee commented May 7, 2024

Chatted with the team. We're pretty excited for this feature, and we think the approach you're taking -- putting the changes in a separate component -- makes this work pretty well. That keeps it safe from conflicts during its development.

So: if you're keen to keep working on this, we absolutely welcome it!

@mary-ext
Copy link
Contributor Author

mary-ext commented May 7, 2024

Nice! I'll see if I can figure out a nice way to tackle the image manipulation, since that's really the only major thing blocking this at the moment

@mary-ext
Copy link
Contributor Author

Wrapped up with implementing DMs on my own Bluesky client, so I'm going to pick this up again, rebasing off 8a12b60

@mary-ext mary-ext mentioned this pull request May 22, 2024
11 tasks
@mary-ext
Copy link
Contributor Author

Closing in favor of #4163

@mary-ext mary-ext closed this May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants