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

Add file uploading support #2552

Merged
merged 1 commit into from Sep 3, 2018

Conversation

@MaxLeiter
Member

MaxLeiter commented Jun 12, 2018

Seems to be pretty ready to go.

UI can be improved in another PR.

Show outdated Hide outdated defaults/config.js
Show outdated Hide outdated src/server.js
Show outdated Hide outdated client/js/upload.js
Show outdated Hide outdated client/js/upload.js
Show outdated Hide outdated client/js/upload.js
Show outdated Hide outdated src/server.js
@fnutt

This comment has been minimized.

Show comment
Hide comment
@fnutt

fnutt Jun 12, 2018

Contributor

https://gfycat.com/ZanyUnsteadyAndalusianhorse was linked earlier on IRC. Shows the lovely uploading feature!

Contributor

fnutt commented Jun 12, 2018

https://gfycat.com/ZanyUnsteadyAndalusianhorse was linked earlier on IRC. Shows the lovely uploading feature!

@dgw

This comment has been minimized.

Show comment
Hide comment
@dgw

dgw Jun 12, 2018

Contributor

This excites me so much, even though I probably wouldn't use the feature as-is because of the dependency on The Lounge and its instance's address for serving the images. (I'm all about log integrity, and I currently choose what services I use for file uploads very carefully based on what I think has the best chance of still being around in ten years.)

The future may bring writing to a custom uploads directory (on a network mount, for example) and returning a custom URL based on that, so uploads can go somewhere besides The Lounge's server if wanted/needed. Maybe it will even support a few third-party hosts (Imgur, etc.) and/or standalone self-hosted image/file servers (Pomf, ownCloud, etc.) via plugins, who knows.

My point is, much of the truly heavy lifting is already in this PR, and I'm positively tingling with excitement for the future of this function. 🎉👏 @MaxLeiter! (Thanks to @christer88 for passing along the video clip of this in action, too!)

Man, it's a great time to be an IRC user. My former IRC friends who've jumped on the Discord bandwagon don't know the Renaissance they're missing! (Even if it is services like Discord that are pushing IRC clients to step up… 😉)

Contributor

dgw commented Jun 12, 2018

This excites me so much, even though I probably wouldn't use the feature as-is because of the dependency on The Lounge and its instance's address for serving the images. (I'm all about log integrity, and I currently choose what services I use for file uploads very carefully based on what I think has the best chance of still being around in ten years.)

The future may bring writing to a custom uploads directory (on a network mount, for example) and returning a custom URL based on that, so uploads can go somewhere besides The Lounge's server if wanted/needed. Maybe it will even support a few third-party hosts (Imgur, etc.) and/or standalone self-hosted image/file servers (Pomf, ownCloud, etc.) via plugins, who knows.

My point is, much of the truly heavy lifting is already in this PR, and I'm positively tingling with excitement for the future of this function. 🎉👏 @MaxLeiter! (Thanks to @christer88 for passing along the video clip of this in action, too!)

Man, it's a great time to be an IRC user. My former IRC friends who've jumped on the Discord bandwagon don't know the Renaissance they're missing! (Even if it is services like Discord that are pushing IRC clients to step up… 😉)

@xPaw xPaw added the Type: Feature label Jun 12, 2018

Show outdated Hide outdated src/server.js
@SinZ163

This comment has been minimized.

Show comment
Hide comment
@SinZ163

SinZ163 Jun 12, 2018

Contributor

What happens when a filename conflict happens?

Contributor

SinZ163 commented Jun 12, 2018

What happens when a filename conflict happens?

@MaxLeiter

This comment has been minimized.

Show comment
Hide comment
@MaxLeiter

MaxLeiter Jun 13, 2018

Member

The library automatically renames the file with a number (-0, -1, etc)

Member

MaxLeiter commented Jun 13, 2018

The library automatically renames the file with a number (-0, -1, etc)

Show outdated Hide outdated src/server.js
Show outdated Hide outdated src/server.js
Show outdated Hide outdated src/server.js
Show outdated Hide outdated src/server.js
Show outdated Hide outdated src/server.js
Show outdated Hide outdated client/js/upload.js
Show outdated Hide outdated client/js/socket.js

@xPaw xPaw added this to the 3.0.0 milestone Jun 19, 2018

@xPaw xPaw requested a review from thelounge/maintainers Jun 20, 2018

@MaxLeiter

This comment has been minimized.

Show comment
Hide comment
@MaxLeiter

MaxLeiter Jun 26, 2018

Member

Relies on #2592

Member

MaxLeiter commented Jun 26, 2018

Relies on #2592

@McInkay

Code looks generally good, but I need to do some more testing on actually running it before I approve. I'll hopefully do that tonight. Looking really great, excited to have this in.

Show resolved Hide resolved src/plugins/uploader.js
Show outdated Hide outdated defaults/config.js
Show outdated Hide outdated defaults/config.js
Show outdated Hide outdated package.json
Show outdated Hide outdated src/plugins/uploader.js
@McInkay

Works quite well for a first version. Needs conflicts fixed. Thanks for this :-)

Show outdated Hide outdated package.json
Show outdated Hide outdated package.json

@astorije astorije self-requested a review Aug 23, 2018

@astorije astorije self-assigned this Aug 23, 2018

@xPaw

This comment has been minimized.

Show comment
Hide comment
@xPaw

xPaw Aug 31, 2018

Member

@astorije I fixed your comments.

Member

xPaw commented Aug 31, 2018

@astorije I fixed your comments.

Add file uploading support
Co-Authored-By: Max Leiter <hello@maxleiter.com>
Co-Authored-By: Jérémie Astori <astorije@users.noreply.github.com>

@xPaw xPaw merged commit 9d5de22 into thelounge:master Sep 3, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@MaxLeiter

This comment has been minimized.

Show comment
Hide comment
@MaxLeiter

MaxLeiter Sep 3, 2018

Member

🎉

Member

MaxLeiter commented Sep 3, 2018

🎉

@MaxLeiter MaxLeiter deleted the MaxLeiter:uploading branch Sep 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment