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

Simple caption for any file #8381

Closed
wants to merge 8 commits into from
Closed

Conversation

bilogic
Copy link

@bilogic bilogic commented Sep 10, 2023

  • Changes have been thoroughly tested to not break existing functionality.
  • New functionality has been documented or existing documentation has been updated to reflect changes.
  • Visual changes are explained in the PR description using a screenshot/recording of before and after.
  1. This is a much simpler (thus better) compared to file caption MVP #8363 + a basic mobile friendly UI.
  2. Because my editor has autoformatting, the 1st 2 commits are just to isolate formatting differences in order to clearly show changes in the subsequent commits
  3. It relies on https://github.com/nielsboogaard/filepond-plugin-manage-metadata but I believe there is a bug which I reported here Edit icon duplicates when editing existing upload nielsboogaard/filepond-plugin-manage-metadata#24
  4. Meanwhile, I copied the main source packages/forms/resources/js/components/filepond-plugin-manage-metadata.js with fix into Filament. We can remove this file and depend fully on that package once issues are resolved
  5. Like before, this should support FileUpload, but only works with SpatieMediaLibraryFileUpload for now
  6. If there is an existing custom property caption, it will be displayed when the user clicks on the Edit icon
  7. It also needs a last bit of work to send the metadata through $wire.upload(..., I stopped at just dumping the metadata to the browser logs because the Livewire portion requires Filament convention knowledge which I'm not familar with
  8. I have also disabled this feature by default to avoid impacting anyone else
  9. There is also 1 known issues which I don't think is caused by https://github.com/nielsboogaard/filepond-plugin-manage-metadata:
    • After editing caption of an initial upload, file tries to upload again but disappears from the UI abruptly, see https://imgur.com/a/r0xXUYe
  10. At least for now, the basics of caption reading to the point just before uploading works

@danharrin danharrin added enhancement New feature or request low priority labels Sep 10, 2023
@danharrin danharrin added this to the v3 milestone Sep 10, 2023
@bilogic
Copy link
Author

bilogic commented Sep 10, 2023

I think the persistence code is quite clean too. 2 issues I have are:

1

$this->annotateUploadedFileUsing(static function (SpatieMediaLibraryFileUpload $component, $fileKey, $data) {
$mediaItem = $component->getRecord()->getRelationValue('media')->firstWhere('uuid', $fileKey);
$mediaItem->setCustomProperty('caption', $data); // adds a new custom property
if ($mediaItem->id) {
$mediaItem->save();
}
return $mediaItem->getAttributeValue('uuid');
});

For an initial upload, $fileKey is null, what should be returned? I suspect this is what's causing the file to disappear from the UI abruptly

2

onManageMetadata: async (fileKey, item) => {
let currentCaption = item.getMetadata('caption');
let newCaption = window.prompt('Edit file caption', currentCaption);
if (newCaption && newCaption != currentCaption) {
// line below will cause filepond to re-upload, need a way to set newCaption
// item.setMetadata('caption', newCaption);
return await $wire.annotateUploadedFile(@js($statePath), fileKey, newCaption);
}
},

Re-uploading is inefficient, I'm not sure how to overcome it

@bilogic
Copy link
Author

bilogic commented Sep 11, 2023

I overcome the 2 issues earlier and found another simpler way to do this, thus removed the filepond-plugin-manage-metadata package, it comes complete with UI and persistence

Right now, an initial upload will have the UI will be hidden and captions cannot be added

@bilogic
Copy link
Author

bilogic commented Sep 11, 2023

I managed to get it to a usable state now, of course plenty of room for improvements.

The current caption can be viewed by hoving over the icon and changes if it gets edited without the need to re-upload the whole file.

@wychoong
Copy link
Contributor

it would be a great feature but there are few questions

  1. how does it looks like and is it match Filament's UI/UX, the demo of the package is not working for me, do you have a screen recording of your implementation?
  2. the plugin is for manage metadata, does it really suite for caption purpose or fit spatie media library custom_properties?
  3. would it be better to use FileUpload component's ->imageEditor approach where render a modal with fields which give more flexibility and possible with such api FileUpload->metadata('custom_properties_column, fields: [ 'caption', 'author', ..])`

again no screenshots/recordings so its just based on imaginary, might be wrong

@bilogic
Copy link
Author

bilogic commented Sep 11, 2023

  1. There is a video in the OP, although it was trying to show a bug https://imgur.com/a/r0xXUYe
  2. It is, but it has some issues, so I dropped it
  3. I think it's better to take it step by step, starting with this basic functionality first, the smallest possible change at a time

I'm curious, what did you mean the demo isn't working? Which demo, and what did it do? Not working doesn't tell me much.

@bilogic
Copy link
Author

bilogic commented Sep 15, 2023

@danharrin

Can I ask what to expect going forward? Thanks.

@danharrin
Copy link
Member

I appreciate all PRs, but as you can see we do have a backlog, and new features are not a priority right now. The review process is very time consuming, and its just me.

@bilogic
Copy link
Author

bilogic commented Sep 15, 2023

Ok, got it thanks.

@bilogic bilogic mentioned this pull request Nov 13, 2023
3 tasks
@danharrin danharrin self-assigned this Nov 18, 2023
@danharrin
Copy link
Member

Thanks for your work. That Filepond plugin does seem too unstable to rely on.

I will accept a caption feature if it makes use of the existing imageEditor() feature. We can easily add new inputs to that modal, which could be useful.

If you want to make a new PR that adds inputs to the image editor modal, go ahead. If not, I will implement it at some point in the future myself.

@danharrin danharrin closed this Nov 28, 2023
@bilogic
Copy link
Author

bilogic commented Nov 29, 2023

@danharrin

Sure, happy to look at imageEditor(), but how do we caption non-images?

@danharrin
Copy link
Member

I don't think we should support captioning non-images, I don't think there is a good enough solution in FilePond right now.

@bilogic
Copy link
Author

bilogic commented Nov 29, 2023

Since captions are stored in the table of medialibrary, can I suggest that we offer a few ways to caption files

  1. Add caption editing for images in ->imageEditor()
  2. Use this PR's browser dialog implementation, works for all files, enabled by calling ->simpleCaptionEditor()
  3. If and when filepond is ready, upgrade ->simpleCaptionEditor() to utilize it

@danharrin
Copy link
Member

Maybe just captionEditor() which uses the existing image dialog if it exists

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request low priority
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants