This is a follow-up for #2352155: Remove HtmlFragment/HtmlPage.
dawehner and joelpittet at #2352155-75: Remove HtmlFragment/HtmlPage:
+++ b/core/lib/Drupal/Core/Render/BareHtmlPageRenderer.php @@ -0,0 +1,80 @@ + public function renderMaintenancePage($content, $title, array $page_additions = []) { + if (!is_array($content)) { + $content = ['#markup' => $content]; + } + $attributes = [ + 'class' => [ + 'maintenance-page', + ], + ]; + return $this->renderBarePage($content, $title, $page_additions, $attributes, 'maintenance_page'); + } + + /** + * {@inheritdoc} + */ + public function renderInstallPage($content, $title, array $page_additions = []) { + $attributes = [ + 'class' => [ + 'install-page', + ], + ]; + return $this->renderBarePage($content, $title, $page_additions, $attributes, 'install_page');The classes we'd like those on the templates.
After that there is literally not really a big difference.
Both @tim and @dawehner thinks that the concept of a bare html page should be not be connected with maintenancen or install itself.
Wim Leers at #2352155-78: Remove HtmlFragment/HtmlPage:
I agree that the concept of a "bare html page" should not be connected with the maintenance or install pages directly. I just couldn't think of a cleaner way to do this earlier.
And I still can't.Because those classes that you'd like to see in the template… they can't live in the template, because they don't apply to the
maintenance-page.html.twigtemplate! They are meant to become classes on the<body>tag. And the tag is owned byhtml.html.twig, notmaintenance-page.html.twig. Just likepage.html.twigends up in{{ page }}inhtml.html.twig,maintenance-page.html.twigandinstall-page.html.twigdo, too.Hence this reroll doesn't change that at all.
Also: this is still a big step forward in clarity/simplicity for rendering maintenance and install pages, so I don't believe we should hold this up over that.
dawehner at #2352155-83: Remove HtmlFragment/HtmlPage:
Well, the distinction could be made by the caller itself, well I don't want to block this really important patch for that,
so let's file a follow up and try to come up with a simpler solution in the future.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | simplify_clean_up-2373735-15.patch | 8.91 KB | joelpittet |
Comments
Comment #1
dawehnerLet's clean this up.
Comment #3
joelpittetComment #4
joelpittetComment #5
joelpittetThese two things conflict it seems, can we fix all the places where it's a string or should we leave it as string|array and just leave the conversion with #markup?
Comment #6
dawehnerYeah, let's just replace all the instances.
Comment #7
joelpittetOk, testbot engage!
Comment #8
joelpittetwhoops, Title may not need that just content in the first param's arg.
Comment #9
joelpittetFixed
Comment #10
joelpittetI should read my interdiffs... before I post them.
Comment #13
joelpittetAdding the theme property.
Comment #14
wim leersI don't know why, but this reminds me of Power Rangers :P
I only found two nitpicks:
Shouldn't this be
'maintenance_page'?You changed the implementation to only accept
array(YAY!) but not the docs :)Comment #15
joelpittetDang it was supposed to be a Star Trek TNG reference:P
#14.1 Thanks totally spaced on that one.
#14.2 Yeah I aim for working before changing other things instead of having an idea scrapped and wasting my time:) Now that we have green, lets do that too!
Comment #16
dawehnerAwesome! Thank you for fixing the issues with it @joelpittet
Comment #17
wim leersRTBC++ :)
Comment #18
catchThis is a nice clean-up enabled by a recent critical (#2352155: Remove HtmlFragment/HtmlPage) so fine for what's allowed during beta.
Patch looks great. Committed/pushed to 8.0.x, thanks!