Hello

On the image_style_form() the row in the draggable table which is contain the select list with the new effects has no class "draggable".

CommentFileSizeAuthor
#3 942060.patch1.85 KBsweetchuck

Comments

ygerasimov’s picture

ygerasimov commented

As I understand the bug is on the page of the configuring effects for image style, right? I can't reproduce on current HEAD. All rows (including new ones created) are draggable. Please advise how to catch this bug.

sweetchuck’s picture

sweetchuck
Primary language Hungarian
Location Budapest
commented
Status: Needs review » Active

I can reproduce this issue on current HEAD from CVS.
I have a brand new installation with the standard profile.
goto ?q=admin/config/media/image-styles/add
Style name = foo
redirected to ?q=admin/config/media/image-styles/edit/foo
add an effect (desturate)
Now the first row in the table is the desaturate, with the drag icon.
Second row (last) is the select list _without_ the drag icon. Missing the draggable class from the <tr> tag.

I can drag the row of desaturate under the select list, but I can't move back above.

In the form builder function image_style_form() not initialize the $form['effects']['new']['weight']['#access'] because
the $form['effects']['new'] element only accessible if the image style is not a default style.
But the theme_image_style_effects() function check this
!empty($form[$key]['weight']['#access'])
this is always FALSE where the $key is equal to 'new'

sweetchuck’s picture

sweetchuck
Primary language Hungarian
Location Budapest
commented
StatusFileSize
new 942060.patch1.85 KB

This patch is wrong. Sorry.
The right line is:

'class' => !empty($form[$key]['weight']['#access']) || $key === 'new' ? array('draggable') : array(),
ygerasimov’s picture

ygerasimov commented
Status: Active » Needs review

I confirm this bug and patch #3 fixes the problem.

nevergone’s picture

nevergone
Primary language Hungarian
commented
Status: Active » Reviewed & tested by the community

This is bug is exist.
The patch is correct and repairs the bug.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

While this breaks UX freeze by making the add row draggable, it does fix the bug indicated in #2. Since the "add row is draggable" pattern is also used in field UI, one could make the argument that its introduction is a bug fix here as well.

So Committed to HEAD, thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.