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

desktop: allow paste-to-post for images in galleries #4584

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

patosullivan
Copy link
Member

@patosullivan patosullivan commented Mar 28, 2025

fixes tlon-3863

At first, I attempted to implement this in app/ui/components/Channel/index.tsx, but I ended up implementing this in ChannelScreen using useUploadState directly and not using the attachment context because:

1) It avoids race conditions between attachment state updates and upload operations (calling attachAssets and waitForAttachmentUploads from the attachment context in succession was fraught).
2) It works within component hierarchy constraints (ChannelScreen is a parent of AttachmentProvider and it feels like this functionality should be in ChannelScreen).
3) It provides granular control for auto-posting each image as a separate message immediately after upload completion

Open to ideas if anyone can think of a better approach, though.

I added a new method to our attachment context that handles this case.

Copy link

linear bot commented Mar 28, 2025

@dnbrwstr
Copy link
Contributor

Would it be possible to extend or modify the attachment context to work better here? This seems like something it really should be able to handle.

@patosullivan
Copy link
Member Author

patosullivan commented Mar 28, 2025

Would it be possible to extend or modify the attachment context to work better here? This seems like something it really should be able to handle.

@dnbrwstr maybe we could add a new method to AttachmentState? uploadAndAutoPost or similar.

…s, fix issue with status type mismatches on UploadImageAttachment
@patosullivan
Copy link
Member Author

@dnbrwstr updated to use the context

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.

2 participants