feat: message images #152
feat: message images #152
Conversation
codegod100
force-pushed
the
message-images
branch
from
4e8f018 to
6047ca4
Compare
codegod100
force-pushed
the
message-images
branch
4 times, most recently
from
1cca237 to
78aa4f7
Compare
|
there is some debug stuff in here was trying to troubleshoot some message issues |
|
Still catching up on stuff, will try to get to this soon. |
|
changing to draft, gonna refactor this to do one thing. too much stuff in here. |
codegod100
force-pushed
the
message-images
branch
from
78aa4f7 to
7c7201d
Compare
|
updated |
|
updated to remove conflict |
|
Going to take a look at this in a bit. |
There was a problem hiding this comment.
Radical! This is a great addition. Having a little more polish on the image that's added to your input would be great. We could also follow this up with another PR to do that. Thanks so much. You really are a @codegod100.
|
thanks for feedback, i'll work on suggestions |
codegod100
force-pushed
the
message-images
branch
2 times, most recently
from
e5cb56f to
3938a72
Compare
|
fixed issues and added drag-and-drop |
codegod100
force-pushed
the
message-images
branch
from
3938a72 to
ed7b6f4
Compare
codegod100
force-pushed
the
message-images
branch
from
ed7b6f4 to
7d9d0a4
Compare
|
You're wild. I'll take a look tomorrow evening. |
|
hold off on review i need to make sure this doesn't conflict with main now that other pr merged |
|
ok issues resolved |
There was a problem hiding this comment.
Awesome addition @codegod100. I think this looks really good and behaves much nicer now. There are a few ideas I have for enhancements down the line that I'll turn into an issue if you want to take a look later on.
src/lib/components/ChatInput.svelte
Outdated
| let isDragOver = $state(false); | ||
|
|
||
| // Consolidated image file processing logic | ||
| async function processImageFile(file: File, input?: HTMLInputElement) { |
There was a problem hiding this comment.
Can't help but wonder if we should call processImageFile after the user submits the message and just show the image in the browser beforehand. I guess I'm worried about uploading a bunch of images and storing them that are never attached to a message. Probably fine for now but perhaps worth a comment to clean up "dangling images" later.
There was a problem hiding this comment.
I guess I'm worried about uploading a bunch of images and storing them that are never attached to a message
I'm having some difficulty parsing this statement. do you mean, someone uploads an image and then in message input field they backspace and delete the image from the message?
There was a problem hiding this comment.
yes, exactly. So they never actually post it.
There was a problem hiding this comment.
We could render the image in the browser, locally, without uploading it, until they submit the message is what I was trying to say.
There was a problem hiding this comment.
the upside is if they do that, the image doesn't go anywhere except into their own pds. we don't store the images in our crdt
There was a problem hiding this comment.
Totally happy with this being a refactor later, or something we deal with in some other way. We could even have a cron that takes care of cleaning things up from time to time.
There was a problem hiding this comment.
if they upload an image and don't use it I don't see why we need to clean it up? if they really care that much can't they use something like https://pdsls.dev? Having a task on our end that manages their pds seems a little out of scope
There was a problem hiding this comment.
We could render the image in the browser, locally, without uploading it, until they submit the message is what I was trying to say.
i understand now, i can refactor this for sure
There was a problem hiding this comment.
if they upload an image and don't use it I don't see why we need to clean it up? if they really care that much can't they use something like https://pdsls.dev? Having a task on our end that manages their pds seems a little out of scope
Ah! I didn't realize we weren't storing it. Then great! No need for the clean up. Especially since we shouldn't just be randomly removing things from people's PDSs.
There was a problem hiding this comment.
We were gonna store in CRDT but many reasons to use pds instead. Top two were csam moderation and CDN
|
ok refactored to only upload image after submit |
|
resolved merge conflicts, fixed regression |
There was a problem hiding this comment.
Looks like the lock file was deleted. deno install again, but it might have some significant changes. Might need to revert this commit instead.
added message image functionality back in