Problem:

On the Color module form, we can "lock" colors in the palette together. This still works, but the visual feedback is broken. The lock icon displays "locked" (yellow, locked icon), but the visual queues ("lines") indicating which fields are locked together don't appear in the correct place, or not at all.

Bug happens in Chrome and FF on Linux.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

wadmiraal’s picture

Assigned: Unassigned » wadmiraal

Checking it out.

wadmiraal’s picture

Title: Color module locks don't show up correctly » Color module lock hooks don't show up correctly
wadmiraal’s picture

Status: Active » Needs review
FileSize
498 bytes

The problem is the scoping of the i variable on line 210 in color.js. In Drupal 7, this variable is correctly scoped, although it does bear the same name as a "global" variable (not really; bound to the attach() scope).

It would probably be nicer to rename this variable to something different to avoid confusion (k ? j is taken), but this will conflict with the patch in #2470069: Refactor color module CSS inline with our CSS standards.

Patching using the same variable name for now, but I suggest we give it a different name.

wadmiraal’s picture

Assigned: wadmiraal » Unassigned
idebr’s picture

Issue tags: +JavaScript
droplet’s picture

Component: CSS » javascript
Status: Needs review » Reviewed & tested by the community

the bug was introduced in #2389515: Update ESLint rules. I couldn't see any reason to make it global. Patch looks fine.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: drupal_color-module-lock-bug_2470769_3.patch, failed testing.

droplet’s picture

Status: Needs work » Needs review
FileSize
551 bytes
Manjit.Singh’s picture

Capture some before and after screenshots.. UI looks good to me.

alt

alt

alt

alt

I have checked functionality also.. No regression issue in functionality. Worked as previous.

nod_’s picture

Status: Needs review » Reviewed & tested by the community

Le sight.

Patch fixes the issue. Ideally we'd rename that but a one liner is good too.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Manually tested and it works - nice. Committed 24c73b4 and pushed to 8.0.x. Thanks!

  • alexpott committed 24c73b4 on 8.0.x
    Issue #2470769 by wadmiraal, droplet, Manjit.Singh: Color module lock...

Status: Fixed » Closed (fixed)

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