Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Part of #2073819: [META] Remove direct calls to drupal_add_css().
Follows the same pattern that was used elsewhere, e.g. in #1969916: Remove drupal_add_js/css from seven.theme.
Comment | File | Size | Author |
---|---|---|---|
#15 | interdiff.txt | 1.88 KB | Wim Leers |
#15 | 2095335-ajax-15.patch | 6.39 KB | Wim Leers |
Comments
Comment #1
Wim LeersThis is inside test coverage, so this is guaranteed to work. Only the code style needs to be reviewed.
Comment #2
Wim LeersI found even more to be removed.
Comment #3
tstoecklerLooks good.
Comment #4
Wim LeersComment #5
Wim LeersThe last two hunks should set
#attached
on$form
.Comment #6
cosmicdreams CreditAttribution: cosmicdreams commentedFrom AjaxResponse->ajaxRender:
This should be handled as well.
Comment #7
Wim Leers#6: no, see #2073819-4: [META] Remove direct calls to drupal_add_css() — this issue is solely about removing
drupal_add_css()
calls that are used to add assets. Not for any others, like these, which are essentially "let's see which CSS and JS were added to this page". They're a different problem space, one that we can only fix once all calls that add assets are gone! :)Comment #9
xjmComment #10
vijaycs85Re-roll + addressing #5
Comment #12
vijaycs85Locally test is fine. So 10: 2095335-ajax-10.patch queued for re-testing.
Comment #14
vijaycs85Sorry testbot, it was my fault...
Comment #15
Wim LeersWas going to RTBC this, but found three very small nitpicks. Fixed those. I don't think they should prevent me from RTBC'ing.
Comment #16
vijaycs85Changes in #15 looks good to me, so +1 for RTBC.
Just a question:
As per #5, addd this line. Can we re move drupal_render() and keep $form['#attached']? will that make any difference? or better way of doing?
Comment #17
Wim Leers#16: I tried doing that, but that caused the tests to fail. It'd be cleaner, so I'd much prefer that, but I couldn't get it to work (I really tried), and it doesn't make sense to hold this patch up on that, which is why I'm just pressing ahead. In any case it's a step forward.
Comment #18
alexpottCommitted ff4a8e9 and pushed to 8.x. Thanks!
Comment #19
Wim Leers