Problem/Motivation
Drupal 8
1. Install 8.9
2. Install Garland
(contributed theme)
3. Set Garland
as default theme
4. Adjust colors and look at preview
5. Preview matches colors chosen except Header top
and Header bottom
6. Save
7. View home page and look at header
8. None of the chosen colors are shown (different bug than this issue)
TBD if this is an issue with the color module or with the contributed Garland
theme.
Note that the colors appear to work fine with Bartik
.
Drupal 7
While testing #14 in #776684: The color.module preview HTML is hardcoded I found the following behavior when choosing color header top and bottom in the Garland theme.
1. Enable color module
2. In Garland settings choose a header top and header bottom color paying attention to what preview.png looks like
3. Save
4. Gradient is different from preview
I'm not sure if this is a 6.x issue. I don't use color.module much.
Proposed resolution
Drupal 8
See below:
Drupal 7
See comment #1 by @webchick for a start of a patch. May be some documentation changes depending on Drupal 8 resolution.
Remaining tasks
- Update issue summary with problem & resolution/approach for Drupal 8 after reviewing color module behavior.
- Write patch for Drupal 8
- Manually test patch for Drupal 8
- Write a beta evaluation
- Determine fix for Drupal 7
- Write a patch for Drupal 7 Garland theme
- Manually test patch for Drupal 7
User interface changes
Color preview will match the colors chosen with the color options.
API changes
N/A
Comment | File | Size | Author |
---|---|---|---|
#21 | garland-header-actual.png | 12.82 KB | kfitz |
#21 | garland-webkit-rendered-preview.png | 15.12 KB | kfitz |
#6 | color-module-test-d7.png | 335.82 KB | balsama |
#6 | color-module-test-d7-after-patch.png | 11.15 KB | balsama |
#6 | header-gradient-preview-778232-6.patch | 606 bytes | balsama |
Comments
Comment #1
webchickThis might be the start of a patch...
Comment #2
tim.plunkettHopefully something can be backported.
Comment #3
markhalliwelltagging
Comment #4
balsamaGarland isn't available in D8, so I tested this with Bartik and could not reproduce it. Screenshots below:
Comment #5
markhalliwellThanks @balsama! Feeling like Christmas already hehe
Alright, since 8.x is fine, let's bump this down to 7.x. Added tags back, I would like to see the same type of wonderful manual testing in 7.x for both Garland/Bartik. It may be theme and not color.module specific.
Comment #6
balsamaIt does seem to still be a problem with Garland:
webchicks suggested patch above does affect Garland's preview, but it's unclear to me if this is the desired result since it's not really a gradient, but more like bands of color:
The attached patch does not affect D7 Bartik or the actual presentation (not the preview) of eithe Bartik or Garland.
Comment #7
oadaeh CreditAttribution: oadaeh commentedProbably the reason why it shows up for Garland and not Bartik is because they each use the Color module differently.
As of 7.x, the color module has two methods of manipulating the colors. One is strictly code based, which is what Bartik uses, and the other is image based, which is what Garland uses.
Not having Garland in 8.x and not being able to easily test it there does not mean this behavior is not present in 8.x.
I will see what I can do to figure this out.
Moving back to 8.x.
Comment #8
oadaeh CreditAttribution: oadaeh commentedThis issue is now dependant on #2380641: The Color module uses filenames as array keys for image slices.
Comment #9
oadaeh CreditAttribution: oadaeh commentedI guess this issue is also dependent on #2380643: Garland breaks whenever the colors are changed even though that's in contrib space.
Comment #10
mgiffordThere has been no new work on this issue in quite some time. So I'm assuming the person assigned is no longer being actively pursuing it. Sincere apologies if this is wrong.
Comment #11
ragnarkurm CreditAttribution: ragnarkurm as a volunteer commentedCannot reproduce the bug on 8.x-dev on Bartik.
This is only standard theme that uses Colors module.
Changed all colors.
They appear correctly on preview and in theme in action.
I assume this not problem for 8.x.
Comment #12
oadaeh CreditAttribution: oadaeh as a volunteer commented@ragnarkurm, please see my comment #7. It is only not a bug for Bartik, because Bartik doesn't use the code that causes the behavior. It is still potentially a bug for Drupal, because the code does exist, but I can't test, prove (or disprove) and fix it, until the issue I linked to in #8 is fixed.
Comment #13
ragnarkurm CreditAttribution: ragnarkurm as a volunteer commentedSorry, I was too quick on this.
I spent some hours on #8 issue, hopefully it will get resolved soon.
Comment #14
alexpolicastro CreditAttribution: alexpolicastro as a volunteer commentedI am at drupalconla and am going to try to replicate the issue.
Comment #15
alexpolicastro CreditAttribution: alexpolicastro as a volunteer commentedComment #16
alexpolicastro CreditAttribution: alexpolicastro as a volunteer commentedI also could not reproduce this problem. Just like others have pointed out Garland has been removed from Drupal 8 Core. From the Garland page "Garland was removed from Drupal core as of Drupal 8 as part of the Platform Initiative."
I am recommending we remove this from the Drupal 8 Core bugs.
Comment #17
alexpolicastro CreditAttribution: alexpolicastro as a volunteer commentedComment #18
oadaeh CreditAttribution: oadaeh as a volunteer commentedJust because Garland is not in core, does not mean core's behavior is not broken and should not be fixed.
Comment #19
alexpolicastro CreditAttribution: alexpolicastro as a volunteer commentedSorry about that, if I'm pretty new at this but then shouldn't it be removed as a Drupal 8 Core issue? As its not in core?
(Only set it to closed as recommended by my DrupalCon mentor)
Comment #20
mradcliffeI think the issue summary is in need of an update as I originally wrote up the issue for Drupal 7, and we're looking into the bug in Drupal 8 color module and its implementations.
I added the issue summary template, but the next step would be to look into color module behavior in Drupal 8, and see if a theme can botch the implementation of the gradient like Garland. Perhaps this is a documentation update for Drupal 8 and a fix for Drupal 7?
Cleaned up some tags and downgraded to Minor for Drupal 8, but probably a Normal issue for Drupal 7.
I hope that helps clarify what the next steps are if you want to continue working on this issue, @alexpolicastro.
Comment #21
kfitz CreditAttribution: kfitz commentedI've been looking into the cause of this issue, and it's caused by Garland using module.color.color.js to render the preview. The Bartik Theme uses the '-webkit-gradient' or '-moz-linear-gradient' method for rendering the gradient. Garland, on the other hand, taps into the functionality of module.color.js and the result is to produce a preview that is not representative of the resultant header gradient.
I am pretty sure the preview that is produced is not a bug in the sense of something being broken, as it appears the color.js is producing the preview it's programmed to. The approach is to produce 10 div's with incrementing id attributes, i.e. "id='gradient-x'", and slightly adjust the colour spectrum values from the top header colour selection to the bottom for each div.
I can get the Garland Theme Preview to successfully render using the same approach as the Bartik Theme, i.e using '-webkit-gradient' or '-moz-linear-gradient' to do the job:
https://www.drupal.org/files/issues/garland-webkit-rendered-preview_1.png
https://www.drupal.org/files/issues/garland-header-actual.png
Note that neither the Bartik or Garland Themes previews render properly in IE8 or IE9, but do work in IE10.
My recommendation would be to refactor the Garland Theme to use the same approach as the Bartik theme for rendering the header preview. I believe I could create the patch for this with out too much difficulty.
Comment #22
kfitz CreditAttribution: kfitz at Acro Commerce commentedComment #23
mradcliffeNice work, @kfitz. I assume that the lack of IE8 support is still a bug because Drupal 7 supports IE8 although a minor bug because it could be worked around. It's probably up to the maintainer whether or not the patch should be required to modify the IE stylesheet or not.
Comment #25
cilefen CreditAttribution: cilefen commentedThere doesn't seem anything immediately actionable by a novice.
Comment #33
Kristen PolTriaging for Bug Smash Initiative.
Comment #34
Kristen PolLooking through the comments, I see that several contributors have tried to bump down this issue from D8 to D7 but it continues to be moved back to a D8 issue even though no one can reproduce the issue on D8. I understand that this may, in principle, be an issue within D8 but given 1) no one can reproduce it and 2) the age of this issue, I would again bump this issue down to D7. I will double check with the bugsmash team on this but IMO it makes sense.
Comment #35
Kristen PolFeedback from xjm (copied with permission):
so bumping back to 8.9 for testing with Garland again to see if it's still an issue.
Comment #36
Kristen PolI've tested Garland on 8.9 and see the issue with color not changes in the preview for Header top and Header bottom but works for the other colors. But the site isn't showing the colors at all for Garland even after clearing the cache. All the colors for Bartik seem to be working. See screenshots. I've updated the issue summary.
Garland:
Settings & Preview:
Home Page:
Bartik:
Settings:
Preview:
Home Page:
Comment #40
quietone CreditAttribution: quietone at PreviousNext commentedColor has been removed from core, #3270899: Remove Color module from core.