Problem/Motivation

theme_system_powered_by() is not a good fit for being overrideable theme output. It returns a simple string of markup, and if you want your own "powered by" block you can just create a custom block or even use string overrides for that matter.

Proposed resolution

Remove theme_system_powered_by().

Remaining tasks

Patch needs review.

User interface changes

None

API changes

Removal of theme_system_powered_by()

CommentFileSizeAuthor
#6 2151117-6.patch1.57 KBstar-szr
#4 2151117-1-twig-theme_system_powered_by.patch1.34 KBjoelpittet

Comments

star-szr’s picture

star-szr
he/him
Primary language English
commented

I'm struggling to understand why this is even a theme function, actually. If you wanted to customize this wouldn't you just make your own block, or even string override for that matter?

star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue summary: View changes

Adding a commit message to the issue summary so the folks who already worked on #1987410: [meta] system.module - Convert theme_ functions to Twig and #1898454: system.module - Convert PHPTemplate templates to Twig (comment #41 and above aka @c4rl) get credit.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

@Cottser totally agree. Block sounds the best... would this be possible to kill, is there a reason behind it? Looks like legacy cruft.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Active » Needs review
StatusFileSize
new 2151117-1-twig-theme_system_powered_by.patch1.34 KB

Though I'd rather delete this... here's the split from the meta.

star-szr’s picture

star-szr
he/him
Primary language English
commented

Well the only use of this theme function is in the powered by block:

http://api.drupal.org/api/search/8/SystemPoweredByBlock

So it seems like we could just move the code there.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Title: Convert theme_system_powered_by() to Twig » Remove theme_system_powered_by()
Issue tags: -Twig, -Needs manual testing, -needs profiling +theme system cleanup
StatusFileSize
new 2151117-6.patch1.57 KB

(New patch so no interdiff.)

So this theme function was originally added in #183332: Add an optional 'Powered by Drupal' block.

While I like the following thought from @eaton, I'm not sure any themes out there actually do this (a survey of the 25 most popular themes from https://drupal.org/project/project_theme says nope!).

An interesting side note. This means that theme developers interested in promoting themselves with a "Designed by so-and-so" blurb can stick that stuff in an overridden theme_system_powered_by(); It would never show up for folks who've already decided they don't want the promotional block.

So I'm not seeing the real use case for providing the ability to override the output of this block via a theme function.

Markup before:

<div class="block block-system contextual-region" id="block-bartik-powered" role="complementary">
  
    <div data-contextual-id="block:block=bartik_powered:"></div>

  <div class="content">
    <span>Powered by <a href="http://drupal.org">Drupal</a></span>
  </div>
</div>

Markup after (it's identical):

<div class="block block-system contextual-region" id="block-bartik-powered" role="complementary">
  
    <div data-contextual-id="block:block=bartik_powered:"></div>

  <div class="content">
    <span>Powered by <a href="http://drupal.org">Drupal</a></span>
  </div>
</div>
star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue summary: View changes

Updating the summary to reflect the proposed patch.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

I agree wholeheartedly with this change.

If further changes are to be made... like maybe make the text editable in the block by default a follow-up could be made but this is still changeable with string overrides.

Markup is identical and works as advertised.

catch’s picture

catch
he/him
Primary language English
commented
Title: Remove theme_system_powered_by() » Change notice: Remove theme_system_powered_by()
Priority: Normal » Major
Status: Reviewed & tested by the community » Active

Committed/pushed to 8.x, thanks!

Needs a very short change notice.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Title: Change notice: Remove theme_system_powered_by() » Remove theme_system_powered_by()
Priority: Major » Normal
Status: Active » Fixed

Thanks! Change notice:
https://drupal.org/node/2153753

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.