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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ygerasimov’s picture

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

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

FileSize
1.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

Status: Active » Needs review

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

nevergone’s picture

Status: Active » Reviewed & tested by the community

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

webchick’s picture

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.