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

Replace plupload with uppy #30

Merged
merged 18 commits into from
Jan 21, 2024
Merged

Replace plupload with uppy #30

merged 18 commits into from
Jan 21, 2024

Conversation

DreamlandOwO
Copy link
Contributor

@DreamlandOwO DreamlandOwO commented Jan 19, 2024

So what I did is I replaced Plupload with Uppy.

Plupload is okay but it doesn't upload multiple files at same time. Both are in simliar download size, so that's good. Uppy supports more ways to upload your files, even from a webcam if we really want that 😂.

Added drag'n'drop to upload modal, if we can just add that to explorer it would be even better.

Upload performance is noticeable better than plupload when there's a lot of files in queue, thanks to Uppy XHR plugin's parallel upload support. Currently I hard-coded parallels limit to 5, maybe we can expose this param to user.

Also some additional tweaks, like:

  • Limit LazyLoad only apply for selectorArea instead of whole document.
  • Added scrollbar to file list on some modal, so it would not expand to the entire page (I tired select over 30 files and delete them, that file list was so long, it blocks the confirm button).
  • Added some preview files so newcomer can just run npm run dev to get things to start.
  • Two new locales, but we need update other locales except English and Chinese.
  • Right way (I think) to filter out external dependencies, so the whole thing would be smaller.

@n1crack
Copy link
Owner

n1crack commented Jan 19, 2024

Nice one, thanks. I was also planning to replace plupload, but couldn't decide which one to replace with.

I'll check the code asap and merge it.

Thanks for the contribution.

@n1crack
Copy link
Owner

n1crack commented Jan 20, 2024

I made some changes, I would be happy if you check it.

  • I think there is no rush for the missing fields in the languages, the relevant people will add them anyway.
  • fixed hebrew shortcode in dropdown.
  • tw preflight mode is turned on again.
  • border color added to the select element in the status bar
  • updated .setSelectable to .setSettings // ".setSelectables" is deprecated and will be removed soon.
  • updated lang-selector width, show full width when mouse over.

@DreamlandOwO
Copy link
Contributor Author

DreamlandOwO commented Jan 20, 2024

Fixed upload clear only successful button, I changed too many times forgot that still broken xD.

One issue with firefox, the <select> by default have a gray background under it, but chrome doesn't have, but yea I mean is firefox we can leave as is.

Chrome:
image

Firefox:
image

Rest looks good to me.

@DreamlandOwO
Copy link
Contributor Author

Yea is kinda dumb show entire message from the server response on the file status, but other issue I encountered is if I try put them into the below messagebox it would be super long, should I just make message box scrollable and file status as simple [Error]?

@n1crack
Copy link
Owner

n1crack commented Jan 20, 2024

Nop its all good, There 2 issues im looking right now.

  1. dark mode colors.
  2. localization import for uppy :) (we can match the lang keys with uppy.)

image

@DreamlandOwO
Copy link
Contributor Author

DreamlandOwO commented Jan 20, 2024

So I made some changes now it looks better on dark mode.

Tailwind for me is very painful deal with, is this the way future devs gonna really writing this for now, back in before CSS is thing? I add some line breaks on class so I can actually knowing which part is which, we should extract them as @apply or components, otherwise this is hell to maintain.

Currently looks like this on upload modal, should apply for other stuff:

image

I did dynamic import uppy locales but it just give me ONE HUNDRED FILES on build, we need figure out a better way to do this.

image

@n1crack
Copy link
Owner

n1crack commented Jan 20, 2024

updated localization can you check it..

  • changed json files to js.
  • we can use uppy's translations for some of the files. I left an example in the ModalUpload

@DreamlandOwO
Copy link
Contributor Author

Oh yea just import uppy locale from our locale is definitely better, we should do that

@n1crack
Copy link
Owner

n1crack commented Jan 20, 2024

we have a merge conflict :)

@DreamlandOwO
Copy link
Contributor Author

I saw that, let me just handle that right now

@n1crack
Copy link
Owner

n1crack commented Jan 20, 2024

already cleaned up. :)

ok Ill leave to you from here. I'll check it again later. (still need to find a way to filter locales and only selected locales to be compiled)

@DreamlandOwO
Copy link
Contributor Author

Nice!

@n1crack
Copy link
Owner

n1crack commented Jan 20, 2024

Tailwind for me is very painful deal with, is this the way future devs gonna really writing this for now, back in before CSS is thing? I add some line breaks on class so I can actually knowing which part is which, we should extract them as @apply or components, otherwise this is hell to maintain.

I missed this part, yes with this It is very hard to maintain. But when you feel comfortable, you barely need to touch css files again. And you get the things done in just a second.

There are 2 options to simplify it, first one is to use @apply, the other one is to break everything to components like in React.

I think, we should go for using @apply.

@n1crack
Copy link
Owner

n1crack commented Jan 20, 2024

With a very long names and on small screens, the list wasn't looking well. have made some changes :

image

image

@n1crack n1crack merged commit 6cb1b3c into n1crack:master Jan 21, 2024
@DreamlandOwO
Copy link
Contributor Author

Yea we can go for@apply, thx for the merge!

@n1crack
Copy link
Owner

n1crack commented Jan 21, 2024

demo in the vuefinder.ozdemir.be is updated.

  • we should move all upload logic to its own js file.
  • There is a 10MB upload limit in the demo page, it doesn't show that error. (maybe I commented the message component, thats why)

otherwise it seems all good. thanks again.

@n1crack
Copy link
Owner

n1crack commented Jan 21, 2024

https://www.youtube.com/watch?v=Gd5lmAb4Iqw

I'll leave this here. It will help to simplify things later.

@DreamlandOwO
Copy link
Contributor Author

Thank you, I look up later.

@n1crack
Copy link
Owner

n1crack commented Jan 22, 2024

Does this work with multiple vuefinder in a single page? (im not sure about ref="root")
instead using root how about using it like this : (btw if it works don't mind, haven't test it yet, )

  <div class="vuefinder" :ref="props.id">

<div class="vuefinder" ref="root">

@DreamlandOwO
Copy link
Contributor Author

DreamlandOwO commented Jan 22, 2024

It will work, that ref is separated by current component instance, "root" as just a name. Unless you manually use inject that ref (as I did declare with provide), other component can give whatever vnode ref name as root, as long as haven't use that name in the same component before.

Oh god I think my explanation is even more confusing, basically ref is not shared, If u decide to use provide to share it, it only works for children component.

Also I think you mixed reply..

@n1crack
Copy link
Owner

n1crack commented Jan 22, 2024

Yea thats right. Im using vue2 and other libraries with my other projects, and confused a bit. yea its all good :) no worries.

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