Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.
Comment | File | Size | Author |
---|---|---|---|
#9 | after-rtl-color-scheme.png | 83.17 KB | Manjit.Singh |
#9 | before-rtl-color-scheme.png | 83.79 KB | Manjit.Singh |
#9 | after-color-scheme.png | 79.61 KB | Manjit.Singh |
#9 | before-color-scheme.png | 79.08 KB | Manjit.Singh |
#8 | color-2470769.patch | 551 bytes | droplet |
Comments
Comment #1
wadmiraal CreditAttribution: wadmiraal commentedChecking it out.
Comment #2
wadmiraal CreditAttribution: wadmiraal commentedComment #3
wadmiraal CreditAttribution: wadmiraal at educa.ch commentedThe problem is the scoping of the
i
variable on line 210 incolor.js
. In Drupal 7, this variable is correctly scoped, although it does bear the same name as a "global" variable (not really; bound to theattach()
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.
Comment #4
wadmiraal CreditAttribution: wadmiraal commentedComment #5
idebr CreditAttribution: idebr commentedComment #6
droplet CreditAttribution: droplet commentedthe bug was introduced in #2389515: Update ESLint rules. I couldn't see any reason to make it global. Patch looks fine.
Comment #8
droplet CreditAttribution: droplet commentedComment #9
Manjit.SinghCapture some before and after screenshots.. UI looks good to me.
I have checked functionality also.. No regression issue in functionality. Worked as previous.
Comment #10
nod_Le sight.
Patch fixes the issue. Ideally we'd rename that but a one liner is good too.
Comment #11
alexpottManually tested and it works - nice. Committed 24c73b4 and pushed to 8.0.x. Thanks!