-
Notifications
You must be signed in to change notification settings - Fork 39
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
File upload support #189
File upload support #189
Conversation
this makes it easier for users to pick and mix them
Codecov Report
@@ Coverage Diff @@
## master #189 +/- ##
==========================================
+ Coverage 96.06% 96.23% +0.17%
==========================================
Files 5 8 +3
Lines 254 558 +304
==========================================
+ Hits 244 537 +293
- Misses 10 21 +11
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
My god, this is turning into a huge PR. Things left to do now:
|
Half of the codebase used `file_id`, and half of it used `file_key`. It has now been standardised as `file_key`.
That's true, but you covered all the possible combinations and obstacles (file names, file extensions, S3 upload, LocalMedia upload, deleting unused files etc. - I must stop here because list of features is really big :) ) I can only say |
This pull request introduces 2 alerts when merging 9827a44 into 26f0d07 - view on LGTM.com new alerts:
|
This makes it easier for people to override `generate_file_key` if they want to.
It was a bit overwhelming having it in the main media storage page. Most people don't have to worry about it.
Provides some visual feedback when uploading a file.
@sinisaos I'm going to merge this in now. It's not 100% perfect, but the rest can be picked up in future PRs. |
@dantownsend I think this is pretty good. Well done. |
@sinisaos Thanks |
Based on #182
@sinisaos I've done a bunch of work on this.
The main changes are:
Array
,Varchar
, andText
columns can all be used as media columnsYou can see the changes here:
Screen.Recording.2022-07-30.at.22.26.37.mp4
The biggest change from the original PR is it saves a unique identifier for the file in the database, rather than the URL. There are two reasons for this:
I think it's about 80% done. It's a massive feature, but a really cool one.
Things that need doing still:
/api/media-files/
at the moment. It's ignoring themedia_url
setting inLocalStorage
. That needs some thinking about.media_columns
andmedia_storage
arguments inTableConfig
. I changed it slightly (now there's justmedia_columns
, which maps a column to aMediaStorage
) because I can imagine use cases where you want some columns to store files in one place, and other columns to store files in another place. Also, you might only allow images in some columns, and videos in other columns. It still needs a bit of thinking about.PiccoloCRUD
standalone.I removed your image previews from
RowListing.vue
for now. I liked the way they looked, but because we're not creating thumbnails for the images, if the page is showing 100 rows, it will pull down 100 full sizes images, which would slow things down too much. Once we work out thumbnails I would like to add it back.