Part of meta-issue #1788918: [GTD] [META] Prepare for Twig main engine core inclusion until December 1
Following #1290694: Provide consistency for attributes and classes arrays provided by template_preprocess(), we should replace any remaining references to drupal_attributes($attributes) with new Drupal\Core\Template\Attribute($attributes).
| Comment | File | Size | Author |
|---|---|---|---|
| #26 | replace_drupal_attributes-1700382-26.patch | 38.54 KB | cameron tod |
| #23 | replace_drupal_attributes-1700382-23.patch | 38.3 KB | cameron tod |
| #18 | replace_drupal_attributes-1700382-18.patch | 38.54 KB | cameron tod |
| #18 | interdiff.txt | 14.15 KB | cameron tod |
| #16 | replace_drupal_attributes-1700382-16.patch | 47.92 KB | cameron tod |
Comments
Comment #1
Crell commentedThis should be an easy enough task.
Comment #2
effulgentsia commented#1290694: Provide consistency for attributes and classes arrays provided by template_preprocess() went in, so no need for this to be postponed any more.
Comment #3
albert volkman commentedFirst pass. I didn't remove the actual drupal_attributes() array, I just updated the calls and documentation.
Comment #4
tim.plunkettEach file should have
use Drupal\Core\Template\Attribute;at the top and refer to it as just "Attribute" inline.Comment #5
albert volkman commentedComment #6
albert volkman commentedComment #7
tim.plunkettThe code conversion looks good, just some doc issues left:
I'm not sure that the wording makes as much sense now that it's a constructor and not a function.
Same wording issue.
This should just be removed wholesale :)
Same wording issue.
Same wording issue.
Comment #8
albert volkman commentedHopefully this is a bit better.
Comment #9
xjm#8: replace_drupal_attributes-1700382-8.patch queued for re-testing.
Comment #10
k_zoltan commentedComment #11
albert volkman commentedPatch no longer applies. Re-roll against current head.
Comment #12
k_zoltan commentedComment #13
k_zoltan commentedReviewed patch after re-roll. All seems to be fine.
The only place "drupal_attributes()" function remains it the "common.inc" where it is declared (although just as a wrapper for the new "Attributes" class.
Comment #14
k_zoltan commentedComment #15
tstoecklerActually drupal_attributes() itself should also be removed. No need to keep a useless function around.
Comment #16
cameron tod commentedRemoved drupal_attributes(), and rerolled the patch. Looks like some unrelated changes made it in to the last one, which have now been removed.
Comment #17
tstoecklerWhile those are nice, they clearly are not related to this issue. :-)
Comment #18
cameron tod commentedYikes! Hopefully this is ok.
Comment #19
tstoecklerI had read this a couple times before, and I trust the interdiff (thanks for providing that!). RTBC. Cool stuff! :-)
Comment #20
catch#18: replace_drupal_attributes-1700382-18.patch queued for re-testing.
Comment #22
cameron tod commentedI will reroll this today if someone doesn't get to it first.
Comment #23
cameron tod commentedReroll.
Comment #24
cameron tod commentedComment #26
cameron tod commentedwhat the
Comment #27
tstoecklerOnly reroll, back to RTBC.
Comment #28
catchCommitted/pushed to 8.x, thanks!
Comment #29
catchHmm this isn't actually covered in #1727592: Attributes and classes arrays have been combined together into Attribute object - it doesn't mention that drupal_attributes() has been completely removed. Re-opening for the addition to the change notice.
Comment #30
cameron tod commentedAdded notes to the change record, see here: http://drupal.org/node/1727592/revisions/view/2293888/2336148
Hope it's ok for me to close this issue.
Comment #31
effulgentsia commentedThanks. Moving to "fixed" though to keep it in front of people watching open issues. The system will automatically move it to "closed (fixed)" after 2 weeks of no activity.
Comment #32
effulgentsia commentedBack to original priority. "critical" was only for the change notice.
Comment #33
jenlamptonAdding tag.
Comment #34.0
(not verified) commentedUpdated issue summary.