Problem/Motivation
Bartik has several process layer functions that need to be removed or relocated, as we are removing the process layer from core.
Proposed resolution
Remove or relocate the functions.
Remaining tasks
Calls to the color module are in process_html, process_page and process_maintenance page.
Color module has to use hook_css_alter in order to work without the process function in the theme.
User interface changes
None
API changes
Color module doesn't require themes to call _color_html_alter and _color_page_alter anymore.
These two functions have been removed, and themes that still use them will break.
Related Issues
#2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class
#1843650: Remove the process layer (hook_process and hook_process_HOOK)
This issue also covers #1941290: Update documentation (Remove the process layer)
Comment | File | Size | Author |
---|---|---|---|
#17 | 2003814-17.patch | 3.83 KB | star-szr |
#17 | interdiff.txt | 289 bytes | star-szr |
#13 | bartik-remove_bartik_process_layer-2003814-12.patch | 3.93 KB | dasjo |
#13 | bartik-remove_bartik_process_layer-2003814-12-interdiff.txt | 616 bytes | dasjo |
#11 | bartik-remove_bartik_process_layer-2003814-11.patch | 3.78 KB | dasjo |
Comments
Comment #1
mchampsee CreditAttribution: mchampsee commentedtaking on as part of Portland sprint
Comment #2
ltrainWait - we already fixed it - just added the issue to provide documentation.
Comment #3
ltrainComment #4
pmelab CreditAttribution: pmelab commentedRemoved process hooks from bartik.theme.
Removed _color_[html|page]_alter functions.
Added color_css_alter().
Comment #4.0
pmelab CreditAttribution: pmelab commentedAdded additional info about API changes
Comment #5
dasjothe given patch removes the color-altered logo functionality without any replacement.
i'll give this a try.
Comment #6
dasjoattached is a patch that extends the patch from #3 and introduces
color_preprocess_page
to retain the functionality formerly provided by_color_page_alter
.Comment #7
dasjoComment #8
dasjoactually this doesn't look good and appears to conflict with #1843710: Remove template_process_maintenance_page()
Comment #9
dasjoupdating tags
Comment #10
dasjosorry for the noise, actually #7 seems to be ok.
Comment #11
dasjothe attached patch integrates a bartik-related fix from #1843710-4: Remove template_process_maintenance_page() in order to get rid of patches conflicting each other.
Comment #13
dasjoadded a fix related to #12
Comment #14
sbudker1 CreditAttribution: sbudker1 commentedTested @dasjo's patch and seemed to work normally! Traveling through the site there were no apparent changes to the bartik theme. In addition we checked the code and the process functions were gone.
Comment #15
eromero1 CreditAttribution: eromero1 commentedTested @dasjo's patch and it seems to work as desired. There were no apparent changes to the Bartik theme during common site use. The process functions were no longer in the code.
Comment #16
jenlamptonThis patch is now blocking #2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class
Comment #16.0
jenlamptonAdded related issue
Comment #17
star-szrRetitling to reflect what this is doing, if anyone has a better title please update. From my understanding these changes need to be done at the same time otherwise things will be broken (I don't think the patch can be split in two). We are essentially changing the color module's API so Bartik needs to be updated accordingly. Tagging for change notification.
Reviewed the patch and it looks good to me, and tested the color module functionality manually as well.
Just tweaking the patch ever so slightly to not introduce an unneeded blank line before a docblock.
Comment #18
jenlamptonre-reviewed the patch (not that it was necessary, but just in case) and still +1.
Comment #19
alexpottManually tested all looking good.
Committed aaa4e9d and pushed to 8.x. Thanks!
Comment #19.0
alexpottblocker
Comment #20
ekl1773Assigning to self to write change notification...
Comment #21
ekl1773Here's a draft of the change notice. Not sure about the length of the before/after code, would welcome comments on how/if it could be trimmed.
Summary
Before:
After:
Before:
After:
Affects: Module developers and themers
Comment #22
star-szrThanks @ekl1773, that's very thorough :) This change notice should IMO be about color module, not Bartik. I think the key point for this change notice is this:
IMO we can remove the first two before/after code snippets entirely - if someone is curious about the behind the scenes changes they can come to this issue and look at the patch. Otherwise this doesn't affect them, it still works the same as before with less code on their part.
And to trim up the second set of before/after code snippets, I would trim the code sample to only show the parts that hook up to color module - you could also consider renaming the bartik_ functions to MYTHEME_ or similar to make them generic. We should then end up with no code at all in the "after" snippet because no extra code is needed!
Comment #23
ekl1773Created change record: https://drupal.org/node/2031831
Comment #24
star-szrMakes sense to me, thanks so much @ekl1773!
Comment #25.0
(not verified) CreditAttribution: commentednot blocking! :)