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 v2 #4163

Closed
wants to merge 49 commits into from

Conversation

mary-ext
Copy link
Contributor

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

Continuation of #3813, moved to a separate branch since this one is being based over 8a12b60

Actual composer-related changes starting from 36ec7fe, with the newer changes starting bb355d0

UI looks more or less the same as before though

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 (and alt text)
  • Quote embeds
  • Image embeds
  • Post language
    • Including the automatic language detection
  • Gated reply interactions
  • Self-labeling
  • Send the post (demo)
  • Address the TODOs left in comments

Notable changes

  • Embeds are represented very differently now, with only a singular embed field
    • Cons:
      • The reducer code I've written to update the embeds might be rather fragile, though the dispatcher actions wouldn't have looked much different I think.
    • Pros:
      • Easy conversion to actual lexicons, since it's pretty much based around that
      • It's more clear what type of embeds can or can't be added or mixed given the current state
      • It should be easier to add new embed types, like videos or custom GIFs, possibly with a different set of constraints on what can be mixed alongside them
      • More control over styling, particularly around recordWithMedia
      • Easy memoization, embeds don't change often, applying it to the top-level component is enough
      • Tenor GIF embeds are easier to deal with now that they're no longer conflated with external embeds (while still being sent as one)
  • record and external embeds are now handled by React Query, post submission no longer needs to be disabled as we try to retrieve details for the embed, since we can also retrieve them as part of the post submission process itself.
  • record embed can now be added after adding external embed, previously this wasn't possible because record embeds had to go through the same process as external embeds, and doing so would just override the external embed before it settles as a record embed
  • record embeds now show a proper preview for lists and feeds in particular.
  • Labels are stored within image and external embeds, I think right now this makes a lot more sense since the only globals labels that exists are for media embeds only. This also means that labels are no longer persisted when embeds are being swapped for another, which might better match user expectations.
  • Thread gating records are now created in the same operation

Additional questions

  • Should the language and labeling options be moved to the same place as reply gate currently is?
  • How should post languages work across multiple posts? At least from the few I've asked, they want the ability to set different languages per-post, and I honestly agree with that

mary-ext and others added 30 commits May 8, 2024 10:51
@mary-ext mary-ext mentioned this pull request May 22, 2024
11 tasks
@mary-ext
Copy link
Contributor Author

mary-ext commented May 23, 2024

There's still some work to be done here, but I think the additional scope required (like making post languages work as expected, additional UI changes, of that sort) means that I should now pass this off to y'all. cc @pfrazee @haileyok

Happy to have worked on this ✌️

@pfrazee
Copy link
Collaborator

pfrazee commented May 23, 2024

Sick! Look forward to giving it all a look.

@mary-ext
Copy link
Contributor Author

mary-ext commented Jun 8, 2024

Between the many Indonesian users moving in, there are a decent amount of fanfic authors looking for threaded composer and drafts.

I think it might be worth making this happen soon (with drafts being easy to add in afterwards), let me know if there's anything else I can help with on getting this pushed forwards.

Cheers!

@pfrazee
Copy link
Collaborator

pfrazee commented Jun 9, 2024 via email

@koyuawsmbrtn
Copy link

It's been three months and now the Brazilians are flooding the platform. Guess it's time to finally review and hopefully merge. Please.

@saltedlolly
Copy link

Now that video is cooked, how about giving this the push it needs? 🙏

@mary-ext
Copy link
Contributor Author

mary-ext commented Sep 14, 2024

There's been a lot of changes to the composer itself since this PR was made, and I need to find the time to work on rebasing the changes. For now though, I'd recommend just using posteing instead

@mary-ext
Copy link
Contributor Author

Good news is that I should have some time to spare on rebasing this PR, problem is, we'd still need to get #3925 merged first (I'll need to rebase this as well, but if that's blocked, this one's blocked)

@gaearon
Copy link
Collaborator

gaearon commented Oct 27, 2024

Ayy, we're finally getting to parity (aside from UI) in #5962. Huge thanks for getting this started — no way I'd be able to figure out how to do it without your work here.

@gaearon gaearon closed this Oct 27, 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.

5 participants