Add ImageWidget with upload/drop/external and inline/widget capabilities #5607
Conversation
✅ Deploy Preview for plone-components canceled.
|
❌ Deploy Preview for volto failed.
|
|
I have moved the #4142 here, because of the conflicts and the big changes since that pr. |
|
I have considered that is better to return always the URL of the image because it will be easier for developers to use if it is consistent. What do you think @sneridagh? |
Co-authored-by: Steve Piercy <web@stevepiercy.com>
|
I compared against production docs. These look the same. Are they supposed to be the same? I'm not sure what I should expect or look for.
The controls appear broken in the Netlify build: against production: Is the difference expected? Both have a broken image. Should that be fixed? Please let me know. Thank you! |
|
Hi @stevepiercy, @sneridagh added this for the storybook: https://65c09ac45778640008535e9c--volto.netlify.app/storybook/?path=/story/edit-widgets-image--default, in the old pr, that I have moved here. You can see that shows the Image Widget. |
|
@dobri1408 nice! Let's get a core contributor to review code. |
* main: (46 commits) Removed word-break:break-word from contents table tbody (#5749) Fix Link to Item and Aliases view not updating content in multilingua… (#5821) Fix HMR problems, upgrade react-refresh and @pmmmwh/react-refresh-web… (#5833) Uses Plone 6.0.10.1 in tests (#5830) Improve wayfinding for various Volto audiences (#5809) Fix issue with HMR and register the same predicate-less component again (#5832) Reset global Form state onSubmit and onCancel in Add and Edit forms. (#5827) Show validation error message as string instead of list (#5808) Updated build-deps command to check if registry is newer than dist to force rebuild (#5825) Upgrade TSQ to latest, testing deployments (#5824) Release 18.0.0-alpha.16 Release @plone/slate 18.0.0-alpha.9 Release @plone/scripts 3.4.0 Release @plone/registry 1.5.1 Release generate-volto 9.0.0-alpha.8 Release @plone/components 2.0.0-alpha.4 Release @plone/client 1.0.0-alpha.13 Several dependencies updates to 18 (#5815) [components] Pass down the Popover context, if any, in Select (#5823) [Components] Improve build, get rid of lodash, renaming Views directory, deps cleaning, upgrade StoryBook and Vite (#5822) ...
|
@dobri1408 let's try to push this forward. I just merged, we need to fix the tests. |
- Although title gives information on hover previous generation of the image widget didn't use title either or most of our widgets. In order to have some consistent behavior we use thus aria-label
|
I have added back placeholderLinkInput = '', because there are tests that change the placeholder Link Input |
- primary button adds a focus border which is not present on the other buttons breaking consistency.
- Removed brown override from toolbar-inner icons. This way when focused we can see opacity change, like it happens with other buttons - .slate-inline-toolbar.slate-toolbar .ui.basic.buttons have transparent box-shadow. We do this to avoid getting the outline focus on primary button outside of #main thus keeping things consistent.
- It makes it easier to use keyboard navigation and go to other buttons if using tab key plus you might want to clear what you had and enter another text
…eless div wrapper for link-form-container logic
|
@sneridagh I've removed the brown important color toolbar-inner-color.mp4I also had to add this rule ec7c2ea#diff-cf128453bb126c12a2d763cd1afdbaf4d32ba380bd02702d0393f5ea4c666bb1R30 @plone/volto-team this is ready for final review |
There was a problem hiding this comment.
It does look amazing, however, I've found a couple of issues. See the video for reference.
- The image placeholder inside a grid block it's too big.
- The input link position is erratic, and depends on the block you selected or the position of the current block?
Could you please take a look?
Screen.Recording.2024-06-21.at.11.51.49.mov
- we don't want to fit to cover the placeholder svg - Added placeholder class to placeholder - use p tag instead of div for description bringing it in line with the teaser block
|
@sneridagh have another look now regarding the placeholder size when added within a grid and the I have one thing that bothers me which is the difference in placeholder svg when we have a grid grid-placeholder-position.mp4If we remove center then things look better but if we only have teasers then they won't look as balanced |
|
@ichim-david I see... difficult question :/ indeed removing it will look better overall. It's been long time that I wanted to change that (and remove the semanticUI class from there). I like https://react-spectrum.adobe.com/react-spectrum/IllustratedMessage.html but I don't see that we are doing anything extraordinary like in there. |
There was a problem hiding this comment.
I could spend another week on this issue and still find things todo but it's one week I don't have at my disposal.
Accessibility, validation, performance improvements, there are all there waiting to be fixed or improved :).
Since a big portion of the code was ripping the image uploading from object browser to a widget the issues that were with it are mostly still here.
For instance you can drag a file or choose upload and select a file instead of an image and you just get a loading image spinner
The object browser navigation using the keybord only is impossible.
None of these issues are though the fault of this ticket and this work is a start from which we can improve in the future as such I accept this work from my part knowing that we will still have more work todo on polishing Volto.
|
@ichim-david I can't agree more. Let's merge this, since it was the original purpose of this PR. |
* main: (73 commits) Release 18.0.0-alpha.36 Rename missing command Image widget PR as breaking (#6125) Release @plone/slate 18.0.0-alpha.14 Release @plone/registry 1.7.0 Rename Makefile targets (#6104) fix: nonContentRoutes diff path (#6102) Automatically set the label to `03 type: feature (plip)` for PLIPs (#6122) Add ImageWidget with upload/drop/external and inline/widget capabilities (#5607) Ensure that sidebar field will not steal focus when metadata is edited (#5983) Prevent duplicated UUUIDs in inner blocks when copying container blocks (#6112) feat: handle breakList in detached TextBlockEditor (#6106) Make dynamic/live teaser without additional requests (#6024) Fix issue of duplicated blocks upon pasting an image into the Slate E… (#5818) Remove `api` folder. Add `backend` folder using latest backend best practices (#6110) Rename files with wrong extension `js->jsx` when they contain JSX. (#6114) Improve shadowing by including the support for `js->jsx` extensions in… (#6113) issue-5452 markdown shortcut (#5495) Release 18.0.0-alpha.35 Release @plone/types 1.0.0-alpha.16 ...
No description provided.