volto icon indicating copy to clipboard operation
volto copied to clipboard

Provide a simple ImageUploadWidget that can be reused in addons

Open tiberiuichim opened this issue 2 years ago • 13 comments

PLIP (Plone Improvement Proposal)

Responsible Persons

Proposer: Tiberiu Ichim

Seconder:

Abstract

There's no basic image upload component that can be reused in addons to provide a reusable solution for this simple task:

  • As a website editor,
  • when I edit a teaser block that requires an image
  • I want to either pick an existing image or upload a new one from my computer

Motivation

I don't like repeating code, and having 1000 different implementations scattered all over the place.

That, and this abomination:

image

Assumptions

We assume that this widget will be reused by the Hero and Image block. It will be a great improvement in the hero block for sure, as it's not possible to pick an existing image, at this moment. For the Image block, the assumption is that the minor UI changes will be digestible.

Proposal & Implementation

image

I've already started to write this widget, based on the code from the Image block and various other places where we have similar code. In the above screenshot I have added the UI that makes the most sense for that widget. Notice that it's slim enough so that it can fit in a block sidebar, which is a major win. The Image block, right now, has a text input right in the middle of the block. This makes the Image block hard to use in limited width contexts (on small screens, in columns, etc). By replacing that text input with the Link button, which will open the Add Link dialog (the one that's used by the text editor to insert a link), we achieve several things:

  • typing an URL is not the most common use case when linking to an image
  • the most common use cases are: uploading an image or picking an existing image
  • the UX is even awkward for the text input field, in the default Image block. You have to hit to confirm finishing the typing. This is not something that's intuitive.

Deliverables

This widget will be included with the refactored Hero block PR

Risks

The expectation is that nothing will be broken in terms of BBB, at least for the new ImageUploadWidget.

Participants

  • Tiberiu Ichim - developer
  • Krisztina Elekes - developer

tiberiuichim avatar Jan 20 '23 08:01 tiberiuichim

@tiberiuichim Sounds reasonable, but note that @sneridagh started implementing something similar in https://github.com/plone/volto/pull/4142

davisagli avatar Jan 23 '23 01:01 davisagli

@davisagli funny, he didn't mention it when I've discussed my PR with him. I'll try to integrate his work as well, as I've progressed quite a bit with my implementation. https://github.com/plone/volto/blob/9cb1bbf23097de5b9dba7fbcc22e6245af5000e6/src/components/manage/Widgets/ImageUploadWidget.jsx

tiberiuichim avatar Jan 23 '23 06:01 tiberiuichim

Already talked to @tiberiuichim and agreed that we should merge both implementations. I like @tiberiuichim approach. I'd like also to include the ability to search for a content inside the link input. I know we had something similar before, but I think that now is more relevant than ever. I seems that it's a pattern that is well accepted and present in Google products, Confluence, WP, etc...

image image

@tiberiuichim if you are ok, I'd like to add this to the specs of the PLIP.

sneridagh avatar Jan 24 '23 09:01 sneridagh

@sneridagh I think the "search for content" is a different PLIP, as it concerns the AddLinkForm component

tiberiuichim avatar Jan 24 '23 09:01 tiberiuichim

We can move it to another PR/issue/PLIP, but if we are overhauling the ImageUploadWidget, I think it's worth to concentrate all the specs in the same place.

sneridagh avatar Jan 24 '23 09:01 sneridagh

@tiberiuichim https://github.com/plone/volto/issues/4303

sneridagh avatar Jan 24 '23 11:01 sneridagh

@tiberiuichim A couple of small features that I think would go a long way in making this widget re-usable:

  • Ability to enable/ disable certain methods of upload (e.g. don't show the manual text in cases where user aren't allowed to show external URLs)
  • Ability to show/ hide the large icon/ configure what is displayed. This would make it useful in the video block as well as custom blocks (e.g. file upload blocks)

I'm happy to work on these after an initial implementation is complete if we wanted these features and want to get this out the door quicker

JeffersonBledsoe avatar Jan 31 '23 09:01 JeffersonBledsoe

@JeffersonBledsoe I would be very happy if you'd jump in an help. You can find my work in this PR, you could extract it, combine it with Victor's original PR (or place my changes on top of his PR) and let's get a dedicated PR for the ImageWidget, either Victor's PR or a new PR that you would start. (Let's call the widget MediaWidget. But we'd need a coherent way to do the previews).

tiberiuichim avatar Jan 31 '23 11:01 tiberiuichim

@JeffersonBledsoe I would be very happy if you'd jump in an help. You can find my work in this PR, you could extract it, combine it with Victor's original PR (or place my changes on top of his PR) and let's get a dedicated PR for the ImageWidget, either Victor's PR or a new PR that you would start. (Let's call the widget MediaWidget. But we'd need a coherent way to do the previews).

I'd also be willing to work on this during this weeks alpine city sprint. @JeffersonBledsoe Did you already start implementing what @tiberiuichim suggested? If not I'd do that, if thats fine with you.

jackahl avatar Feb 06 '23 17:02 jackahl

I'd also be willing to work on this during this weeks alpine city sprint. @JeffersonBledsoe Did you already start implementing what @tiberiuichim suggested? If not I'd do that, if thats fine with you.

Hey @jackahl. I've not had chance to start this yet so go for it!

JeffersonBledsoe avatar Feb 06 '23 18:02 JeffersonBledsoe

It seems #5607 is the active PR that implements this.

pbauer avatar Jan 10 '24 15:01 pbauer

@sneridagh is this a duplicate PLIP now or shall we keep both of them for the Plone roadmap?

tisto avatar Feb 12 '24 18:02 tisto

@tisto not a duplicate.

sneridagh avatar Mar 05 '24 08:03 sneridagh