Fixes for Image widget #6151

ichim-david · 2024-07-03T17:26:16Z

…dget on a single column

…ate PositionedToolbar

…he container of the trigger button instead of body - This way we don't have problems with position when used within the sidebar for fields of blocks

- it's no point in adding it since it will result in duplicated id's due to this widget being used in more than one context

…de of body and inside image widget area

netlify · 2024-07-03T17:26:30Z

✅ Deploy Preview for plone-components canceled.

Name	Link
🔨 Latest commit	`e989b33`
🔍 Latest deploy log	https://app.netlify.com/sites/plone-components/deploys/66869e0fa20c3a0009f3ba3f

ichim-david · 2024-07-03T17:27:18Z

@sneridagh not done yet, still some cleanup and test failures but this is my result so far

image-widget-relative.mp4

sneridagh · 2024-07-03T17:54:21Z

Let's talk tomorrow and try to merge both PRs on the image widget.

…sed on non Image content type

…slate - After modification made in e5dd823 link button no longer appeared since it wasn't called with setting objectBrowserPickerType which we now add as a default prop. This way it shows up in slate link popup and not within the image widget which correctly passes this prop

…rop or default value for LinkEditor

…dded changelog

…tyles cross browser - In edge the toolbar was a lot smaller than the input area as opposed to FF or Safari

…duplicated css and avoid the sidebar left override.

ichim-david · 2024-07-04T09:05:29Z

I took the fix from @sneridagh regarding the image upload bug but left out he css changes since I have modified the ImageWidget to make the label for FormFieldWrapper and by adding class block to the image-widget-upload we got the styling without need for style duplication.

Here is how the image widget behaves on the image and on a content that might use it, in my demo I appied it to the teaser but you could add it to something else.
Tested in Safari, Firefox and Chrome (suprisingly chrome has some issues with the width of the input which is why I had to add some extra widths statements :( )

image-widget.mp4

And here is the Link plugin button showing back to the Slate toolbar add link

link-widget.mp4

EDIT:
I had to remove the always computing position of the link popup since it was moving around as you've scrolled.
Instead you can pass position to LinkEditor like you could to the PositionedToolbar from slate otherwise it uses a default that places it above the area of the trigger button.

link-popup-position.mp4

sneridagh

LGTM! Thanks @ichim-david for being so thorough, amazing work!

…iggered from Slate toolbar

…rides

- in Safari we got a small shift in sidebar widget at 82%

stevepiercy

It would be nice to use one line per sentence in the change logs. It gets rendered as HTML, so we don't care about line wraps at 80 (or whatever) characters.

stevepiercy · 2024-07-04T12:30:17Z

packages/volto-slate/news/6159.feature

@@ -0,0 +1,7 @@
+Use unused `toggleButton` prop in `PositionedToolbar` to render the 
+toolbar in a different portal target falling back to document.body 


Suggested change

toolbar in a different portal target falling back to document.body

toolbar in a different portal target falling back to `document.body`

stevepiercy · 2024-07-04T12:30:47Z

packages/volto-slate/news/6159.feature

+toolbar in a different portal target falling back to document.body 
+if `toggleButton` is not provided. @ichim-david
+
+When `toggleButton` is provided as portal target, we allow negative


Suggested change

When `toggleButton` is provided as portal target, we allow negative

When `toggleButton` is provided as portal target, allow negative

stevepiercy · 2024-07-04T12:31:15Z

packages/volto-slate/news/6159.feature

+if `toggleButton` is not provided. @ichim-david
+
+When `toggleButton` is provided as portal target, we allow negative
+left positioning but not when target is body to avoid the toolbar 


Suggested change

left positioning but not when target is body to avoid the toolbar

left positioning except when the target is `body` to prevent the toolbar

stevepiercy · 2024-07-04T12:31:36Z

packages/volto-slate/news/6159.feature

+
+When `toggleButton` is provided as portal target, we allow negative
+left positioning but not when target is body to avoid the toolbar 
+going off screen and avoiding breaking changes. @ichim-david


Suggested change

going off screen and avoiding breaking changes. @ichim-david

going off screen and avoid breaking changes. @ichim-david

stevepiercy · 2024-07-04T12:34:37Z

packages/volto/news/6159.breaking

@@ -0,0 +1,4 @@
+Fixed image widget position and look and feel in sidebar @ichim-david


Suggested change

Fixed image widget position and look and feel in sidebar @ichim-david

Fixed image widget position and look and feel in sidebar. @ichim-david

stevepiercy · 2024-07-04T12:41:06Z

packages/volto/news/6159.breaking

+Fixed image widget position and look and feel in sidebar @ichim-david
+
+Breaking:
+- This change updated the markup of the widget in the generated form both in sidebar and in a normal form.


Is there a process that needs to be added to the upgrade guide?

What should developers search for and change, if needed?

In which file should they search for details?

Is there a process that needs to be added to the upgrade guide?

What should developers search for and change, if needed?

In which file should they search for details?

@stevepiercy I modified the text with some information on what changed.
It has todo with the image widget added ast part of Volto 18 alpha 35 or 36
#5607
We found some bugs that needed fixing. I don't know if we need such breaking
for breaking changes, from my point of view being in alpha means that we can
make a fix that changes things added in a preview alpha if we found problems with
the work that we approved and merged then.

If you think otherwise and still need further info added get in touch with me and lets
finish the info as needed.

@stevepiercy @ichim-david Since it's a breaking of a breaking and it's about markup, it's fine, to my understanding no need yet another entry for something that unlikely someone has already customized. We only did that in case an API changing, etc.

OK, that sounds good, just wanted to make sure that breaking is OK in this context.

…stevepiercy

…stevepiercy

ichim-david added 5 commits July 3, 2024 15:33

Modified ImageUploadWidget FormFieldWrapper call to have the image wi…

ddac42e

…dget on a single column

Make use of toggleButton prop to allow different portal target for Sl…

c17d730

…ate PositionedToolbar

WIP setting position statically opting to insert the toolbar within t…

faed6ca

…he container of the trigger button instead of body - This way we don't have problems with position when used within the sidebar for fields of blocks

removed id from input of AddLinkForm

63a4534

- it's no point in adding it since it will result in duplicated id's due to this widget being used in more than one context

style changes for link-form-container and children for add link outsi…

94ba7a3

…de of body and inside image widget area

ichim-david and others added 8 commits July 3, 2024 21:19

Fixed recurrive error when uploading image when the image widget is u…

d22ff16

…sed on non Image content type

Removed calculated position on every scroll, use static position as p…

5668b87

…rop or default value for LinkEditor

Added fix for reccursive image upload error fixed by @sneridagh and a…

d1706f8

…dded changelog

Added history entry to volto-slate.

485c11b

Tweaked link-form-container and slate-toolbar styles after checking s…

1a91758

…tyles cross browser - In edge the toolbar was a lot smaller than the input area as opposed to FF or Safari

Added block class to image-widget-upload. This way we can remove the …

83e715b

…duplicated css and avoid the sidebar left override.

Changelog

0a2ab82

Added breaking notice for volto-slate

27b3f13

sneridagh approved these changes Jul 4, 2024

View reviewed changes

sneridagh mentioned this pull request Jul 4, 2024

Several fixes for imagewidget #6152

Closed

sneridagh changed the title ~~6149 image widget fixes~~ Fixes for Image widget Jul 4, 2024

ichim-david added 2 commits July 4, 2024 13:17

Avoid breaking change and bug to the position of AddLink when it's tr…

75ec956

…iggered from Slate toolbar

Cleaned extra styles no longer needed and added comments for key over…

0fb04b4

…rides

ichim-david requested a review from stevepiercy July 4, 2024 11:55

drop max-width to 81% for anchorlink-theme

3515afb

- in Safari we got a small shift in sidebar widget at 82%

stevepiercy requested changes Jul 4, 2024

View reviewed changes

ichim-david added 2 commits July 4, 2024 15:48

News grammar tweaks

48d3723

Tweaked news information taking into consideration feedback given by @…

e989b33

…stevepiercy

stevepiercy approved these changes Jul 4, 2024

View reviewed changes

sneridagh merged commit 8147643 into main Jul 4, 2024
70 checks passed

sneridagh deleted the 6149_image_widget_fixes branch July 4, 2024 15:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes for Image widget #6151

Fixes for Image widget #6151

ichim-david commented Jul 3, 2024 •

edited

Loading

netlify bot commented Jul 3, 2024 •

edited

Loading

ichim-david commented Jul 3, 2024

sneridagh commented Jul 3, 2024

ichim-david commented Jul 4, 2024 •

edited

Loading

sneridagh left a comment

stevepiercy left a comment

stevepiercy Jul 4, 2024

stevepiercy Jul 4, 2024

stevepiercy Jul 4, 2024

stevepiercy Jul 4, 2024

stevepiercy Jul 4, 2024

stevepiercy Jul 4, 2024

ichim-david Jul 4, 2024

sneridagh Jul 4, 2024 •

edited

Loading

stevepiercy Jul 4, 2024

		@@ -0,0 +1,7 @@
		Use unused `toggleButton` prop in `PositionedToolbar` to render the
		toolbar in a different portal target falling back to document.body

	When `toggleButton` is provided as portal target, we allow negative
	When `toggleButton` is provided as portal target, allow negative

	left positioning but not when target is body to avoid the toolbar
	left positioning except when the target is `body` to prevent the toolbar

	going off screen and avoiding breaking changes. @ichim-david
	going off screen and avoid breaking changes. @ichim-david

		@@ -0,0 +1,4 @@
		Fixed image widget position and look and feel in sidebar @ichim-david

Fixes for Image widget #6151

Fixes for Image widget #6151

Conversation

ichim-david commented Jul 3, 2024 • edited Loading

netlify bot commented Jul 3, 2024 • edited Loading

✅ Deploy Preview for plone-components canceled.

ichim-david commented Jul 3, 2024

sneridagh commented Jul 3, 2024

ichim-david commented Jul 4, 2024 • edited Loading

sneridagh left a comment

Choose a reason for hiding this comment

stevepiercy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sneridagh Jul 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ichim-david commented Jul 3, 2024 •

edited

Loading

netlify bot commented Jul 3, 2024 •

edited

Loading

ichim-david commented Jul 4, 2024 •

edited

Loading

sneridagh Jul 4, 2024 •

edited

Loading