Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
As follow up to #1934420: Allow area handlers to return a render array and as general concept, we should handle the view preview as render array too.
Comment | File | Size | Author |
---|---|---|---|
#9 | interdiff.txt | 2.62 KB | dawehner |
#9 | drupal-1938380-9.patch | 20.76 KB | dawehner |
#7 | 1938380-7.patch | 20.5 KB | damiankloip |
#7 | interdiff.txt | 2.02 KB | damiankloip |
#6 | interdiff.txt | 16.77 KB | damiankloip |
Comments
Comment #1
damiankloip CreditAttribution: damiankloip commentedThere might be an issue here as the preview is used with an AJAX command, and that will expect the result to be rendered. This could be moved somewhere else, just a warning :)
Comment #2
damiankloip CreditAttribution: damiankloip commentedWorking on this one.
Comment #3
damiankloip CreditAttribution: damiankloip commentedWe may have to do something like this, which I'm not sure about.
Comment #4
olli CreditAttribution: olli commentedJust a few notes:
Lots of these, what about
instead?
Indentation.
Obsolete.
Comment #5
damiankloip CreditAttribution: damiankloip commentedThanks for the review! Much appreciated.
Here are those changes, let's see how this gets on. In a bit of a rush so if there are some missing I will have to fi them later :)
Comment #6
damiankloip CreditAttribution: damiankloip commentedAnd the interdiff..
Comment #7
damiankloip CreditAttribution: damiankloip commentedSpotted a couple of things.
Comment #9
dawehnerFixing this bunch of silly mistakes :)
Comment #10
damiankloip CreditAttribution: damiankloip commentedThanks! I did say I ran out of time :)
Comment #11
dawehnerForgot to set the patch rtbc
Comment #12
tim.plunkettRTBC+1
Comment #13
webchick#9: drupal-1938380-9.patch queued for re-testing.
Comment #14
xjmSo Alex and I talked at length about this method, since it was setting a value in
#markup
rather than returning a "true" render array. However, in this particular case, the contents of the#markup
isn't actually markup at all--it's a string representation of the REST response, getting wrapped in a<pre>
tag for display to the user.I did notice that we are calling
drupal_render()
inStylePluginBase::render_grouping_sets()
-- Is that a requirement, or is that something we're planning to clean up later? Out of scope here in any case.So we agree that this is commit-ready once it finishes testing.
Comment #15
dawehnerOpened a follow up: #1962836: Don't use drupal_render in StylePluginBase::render_grouping_sets()
Comment #16
webchickCommitted and pushed to 8.x. Thanks!