Updated: Comment #0
Problem/Motivation
When grepping to determine the scope of #2049207: [Follow up] Replace .tpl.php with .html.twig in documentation it was determined that the "Theming information" functionality in Views still refers to .tpl.php files and is broken.
Proposed resolution
Remove this functionality, twig_debug either already does or will soon replicate this functionality in HTML comments.
Remaining tasks
None
Steps to Reproduce
- Navigate to admin/structure/views/view/content
- Go to Advanced > Output: Templates
- Click on "Field Content: Node operations bulk form (ID: node_bulk_form):"
You should see the contents of the currently used template for rows. Currently this is empty.
User interface changes
The "Theming information" UI will look for .html.twig files instead of .tpl.php files.
Screen shots from #12
Before #9:
After #9:
API changes
n/a
Comment | File | Size | Author |
---|---|---|---|
#25 | views-2049209-25.patch | 11.96 KB | dawehner |
Comments
Comment #1
star-szrThis is a bit different than I thought, and maybe it would be nice to make this code theme engine agnostic.
Comment #1.0
star-szrAdd STR
Comment #2
star-szrRetitling.
Comment #2.0
star-szrAdd screenshot
Comment #3
Manuel Garcia CreditAttribution: Manuel Garcia commentedHope I didn't miss any =)
Comment #4
star-szrThanks @Manuel Garcia :) that gets them all.
Looking at this again it looks possible to be theme engine agnostic, there is some existing code in this class to do this which would be nice to try and reuse. It's in a big switch statement but maybe this code could be moved out into a helper method?
Comment #5
star-szrSomething like this. Interdiff is the same size as the patch, so not uploading it.
Comment #7
lokapujyareroll
Comment #8
Manuel Garcia CreditAttribution: Manuel Garcia commented@Cottser that makes sense =)
@lokapujya thanks for the re-roll!
I'll be bold and set it to rtbc.
Comment #9
star-szrThank you @lokapujya and @Manuel Garcia! The latest patch shrunk a bit, I think we still want to move the code from within the 'analyze-theme' case to the getThemeInformation method.
This is still failing tests locally for me but I don't think we want the repeated code. I just cut the code and pasted it back into getThemeInformation to include some recent changes. Probably something small would explain why this doesn't work but I haven't had a chance to debug. Manual testing might help.
Comment #10
star-szrSomething like this perhaps…
Edit: although, this seems like now the code in the helper function could be run up to 4 times within the analyze-theme case.
Comment #11
star-szrOkay, well #9 is the patch to review then! :)
Comment #12
Tim Bozeman CreditAttribution: Tim Bozeman commentedI followed the "Steps to Reproduce" which originally produced a warning: file_get_contents(...)
After I applied the patch it shows a doc block.
Whats next, code review?
Comment #12.0
Tim Bozeman CreditAttribution: Tim Bozeman commentedClarify expected behaviour a bit
Comment #12.1
Tim Bozeman CreditAttribution: Tim Bozeman commentedAdded screenshots from #12 to issue summary changes.
Comment #12.2
Tim Bozeman CreditAttribution: Tim Bozeman commentedUpdated issue summary.
Comment #13
dawehnerWe seem to have all kind of problems with this functionality.
This fixes at least the overview screen for me. I would like to have someone working on this who actually understands the new theme system.
Comment #14
star-szrI've never used this functionality myself. Another question might be do we need this with twig_debug in core?
Comment #15
dawehnerWell, this functionality is more about creating and finding views templates. I agree that this is someone which is not done anywhere else in core.
Can you refer that twig debug in core would mean. Would you have an easy way to find out which templates would be possible to override?
Comment #16
star-szrYes, when twig_debug is enabled we display all possible template suggestions and show the current one in use (marked with an x).
https://drupal.org/node/1922666
Comment #17
dawehnerViews using passing multiple theme callbacks to #theme, so this sadly does not work.
Comment #17.0
dawehnerfixed screen shots from #12 widths
Comment #18
jibran13: vdc-2049209-13.patch queued for re-testing.
Comment #20
tim.plunkettAnother problem is that #1886448: Rewrite the theme registry into a proper service added
\Drupal::service('cache.theme');
, which doesn't exist. If this had been fixed, that would have been caught.Comment #21
jibranHere is a reroll. Fixed #20. I was unable to access
admin/structure/views/nojs/display/frontpage/page_1/analyze-theme
because of #20. Analyzing the theme is very important from development point of view so bumping it to major.Comment #23
jibran21: vdc-2049209-21.patch queued for re-testing.
Comment #24
tim.plunkettPer #14 and #16, I think we may want to just rip this out completely.
Comment #25
dawehnerIf it will be really common for theme developers to look into the HTML i would be totally fine with it.
Comment #26
damiankloip CreditAttribution: damiankloip commentedYeah, we all (me, Daniel, Tim) talked about this a while back and decided it's good to let this go!
Comment #27
tim.plunkett+1
Comment #28
star-szrRetitling, tweaking the issue summary, and adding a couple related issues.
#2118743: Twig debug output does not display all suggestions when an array of theme hooks is passed to #theme was opened to make sure an array of theme hooks passed to _theme() is displayed in twig_debug output, but that looks like it will be solved by #2201781: Pass all theme hook suggestions to theme preprocess functions to allow for suggestion-specific overrides though.
Comment #29
star-szrI'll post a rough initial patch at #2201781: Pass all theme hook suggestions to theme preprocess functions to allow for suggestion-specific overrides in a few minutes but here's a before/after example:
Before patch
After patch
Obviously we don't want views-view.html.twig there twice, but you get the idea :)
Comment #30
jibranWe need a change record here. It is a simple code removal so not major more.
Comment #31
tim.plunkettIt doesn't need a change notice, we're just removing a bit of functionality that was added in D8.
And it is major, since it's a bug right now, we just happen to be fixing it by removing it.
Comment #32
catchThe debug output looks fine, no point providing this twice.
Committed/pushed to 8.x, thanks!
Comment #33
damiankloip CreditAttribution: damiankloip commentedThis really IS good news!
Comment #34
joelpittetIt has potential to be good news... though I know a number of themers/site builders that rely on that template list including me. twig_debug has the potential to be a more global solution for all templates. We will likely need a way to surface this info and if I remember correctly there may have started a devel_themer style UI to parse twig_debug comments.
So I'm hoping is the controlled burning to spur growth type of removal;)
Comment #35
joelpittetWhoops, crosspost.