Block party #4406

nternetinspired · 2014-09-30T16:53:19Z

A premise, I promise

I think in order to facilitate ease-of-use, readability and maintenance related, and dependant, html should be rendered in one place. This is why we have layouts, after all.

Paints go in a paintbox, pencils in a pencil tin.

Currently, the interdependent markup for the article infoblock is scattered across 8 files. The parent block.php defines the content as a document list and each child element contains the document definition.

The problem? a <dd> can not live a life independent of a <dl>, so these structural markup elements should live together.

Once upon a time…

A young prince (not me) decided that his document information would be better served as a list of items and set out upon a great and noble quest. However, upon reaching the hitherto little-trod realm of /layouts/joomla/content/info-block this young prince did discover he must slay(override) not 1, but 8 fearsome dragons(layout files). This brought great sadness upon our hero and he was avowed to forge, with great cunning, a plan such that none should have to endure these trials again.

🔪 🐉

TL:DR;

This PR pulls related markup from the 8 infoblock overrides into the parent, infoblock.php. This means that any wishing to change the markup, to an unordered list or simple paragraph for example, can do so with just one override.

Testing

Apply patch and check that neither appearance or markup of article info block is changed.

Signed-off-by: Seth Warburton <seth@internet-inspired.com>

brianteeman · 2014-09-30T17:21:19Z

I guess the question is what was the original thinking behind structuring
it this way

On 30 September 2014 17:53, Seth Warburton notifications@github.com wrote:

A premise, I promise

I think in order to facilitate ease-of-use, readability and maintenance
related, and dependant, html should be rendered in one place. This is why
we have layouts, after all.
Paints go in a paintbox, pencils in a pencil tin.

Currently, the interdependent markup for the article infoblock is
scattered across 8 files. The parent block.php defines the content as a
document list and each child element contains the document definition.

The problem? a
can not live a life independent of a
, so these
structural markup elements should live together.
Once upon a time…

A young prince (not me) decided that his document information would be
better served as a list of items and set out upon a great and noble quest.
However, upon reaching the hitherto little-trod realm of
/layouts/joomla/content/info-block this young prince did discover he must
slay(override) not 1, but 8 fearsome dragons(layout files). This brought
great sadness upon our hero and he was avowed to forge, with great cunning,
a plan such that none should have to endure these trials again.

[image: 🔪][image: 🐉]
TL:DR;

This PR pulls related markup from the 8 infoblock overrides into the
parent, infoblock.php. This means that any wishing to change the markup, to
an unordered list or simple paragraph for example, can do so with just one
override.
Testing

Apply patch and check that neither appearance or markup of article info

block is changed.

You can merge this Pull Request by running

git pull https://github.com/nternetinspired/joomla-cms new-kids-on-the-block

Or view, comment on, or merge it at:

#4406
Commit Summary

Block party

File Changes

M layouts/joomla/content/info_block/author.php
https://github.com/joomla/joomla-cms/pull/4406/files#diff-0 (17)

M layouts/joomla/content/info_block/block.php
https://github.com/joomla/joomla-cms/pull/4406/files#diff-1 (72)

M layouts/joomla/content/info_block/category.php
https://github.com/joomla/joomla-cms/pull/4406/files#diff-2 (17)

M layouts/joomla/content/info_block/create_date.php
https://github.com/joomla/joomla-cms/pull/4406/files#diff-3 (11)

M layouts/joomla/content/info_block/hits.php
https://github.com/joomla/joomla-cms/pull/4406/files#diff-4 (9)

M layouts/joomla/content/info_block/modify_date.php
https://github.com/joomla/joomla-cms/pull/4406/files#diff-5 (11)

M layouts/joomla/content/info_block/parent_category.php
https://github.com/joomla/joomla-cms/pull/4406/files#diff-6 (17)

M layouts/joomla/content/info_block/publish_date.php
https://github.com/joomla/joomla-cms/pull/4406/files#diff-7 (11)

Patch Links:

https://github.com/joomla/joomla-cms/pull/4406.patch

https://github.com/joomla/joomla-cms/pull/4406.diff

—
Reply to this email directly or view it on GitHub
#4406.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

nternetinspired · 2014-09-30T19:38:45Z

I personally can't see any good reason for doing it like that.

wilsonge · 2014-10-01T09:21:40Z

I agree with the principle of this change but it worries me if someone has overloaded one of the sub files (to change an icon or something) then the html structure is going to be ruined by this change because they'll have nested <dd> elements

nternetinspired · 2014-10-01T09:40:46Z

Well, their output would then be semantically incorrect <li><dd>something I overrode</dd></li> but there would be no visual impact on rendering, as both ul and dl are list types to which the same browser defaults are applied.

I think we can also safely assume that the number of in-the-wild overrides of the infoblock layouts are pretty limited. Template clubs are still overriding these structures with view overrides AFAIK.

Bakual · 2014-10-01T13:13:43Z

I agree that this JLayout is one of those that aren't coded ideally. It has other shortcomings as well (eg: try to use it in a 3rd party extension add or remove some sublayouts to it. It's all hardcoded).
Seth makes a good point here. But as George sid you can't change it now because you will certainly break some templates while doing it.

Well, their output would then be semantically incorrect <li><dd>something I overrode</dd></li>

I'm quite sure a kitten would die somewhere due to this 😄

I think we can also safely assume that the number of in-the-wild overrides of the infoblock layouts are pretty limited.

Arguments like this are not bringing us anywhere. It either possibly breaks layouts, or it doesn't. If it does break even only for 0.1% of the people, it still means several thousand sites.

But main point is: If you want to override the info_block, you only really need to override the parent one. Change it so it doesn't call any of the childs and you're fine.
The reason why it's broken up into multiple layouts is that it's easy to change one of the children alone and you don't have to change all of them.
But if you want to change the tags used, then it is sure easier to just override the parent and do all your changes there and don't use the children at all.

nternetinspired · 2014-10-01T13:22:46Z

The reason why it's broken up into multiple layouts is that it's easy to change one of the children alone and you don't have to change all of them.

And this PR doesn't change that, It's still possible to change just one child. This PR only ensures that tightly-coupled, and interdependent, markup is rendered in the same place.

It either possibly breaks layouts, or it doesn't.

It doesn't. Worst case scenario is some sub-optimal markup.

But main point is: If you want to override the info_block, you only really need to override the parent one. Change it so it doesn't call any of the childs and you're fine.

That would completely negate the reason for creating child layouts for infoblock in the first place. If that is an acceptable solution then let's just render the entire infoblock in a single layout. That would be my preference.

nternetinspired · 2014-10-01T13:25:23Z

I'm quite sure a kitten would die somewhere due to this 😄

Agree, it would be pretty offensive to me also!

infograf768 · 2014-10-02T07:46:42Z

Travis fails:
FILE: .../build/joomla/joomla-cms/layouts/joomla/content/info_block/category.php

FOUND 1 ERROR(S) AFFECTING 1 LINE(S)

20 | ERROR | Additional whitespace found at end of file

Signed-off-by: Seth Warburton <seth@internet-inspired.com>

nternetinspired · 2014-10-02T09:01:58Z

Trailing space removed.

Note: This PR also adds the closing lines that were not present in child layouts ;)

Hackwar · 2014-11-17T11:57:45Z

I would strongly prefer to have this in one layout as well instead of a dozen files. We are not making our lifes easier by creating large numbers of files.

nternetinspired · 2014-11-17T12:04:03Z

I would strongly prefer to have this in one layout as well instead of a dozen files. We are not making our lifes easier by creating large numbers of files.

Agreed. That's the approach I've adopted for my own overrides. It is after-all a single 'info block', we may as well have the layout as a single block.

Being able to change a given element in a single place was the primary motive for having layouts in the first place…

G-Lu · 2015-08-21T11:05:24Z

Tested the patch on serveral templates and got no differnece in the appearance. (not patched/patched)
In some templates the html-code got some diffrent white spaces and line breaks but the essential code was the same.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4406.}

Blutengel89 · 2015-08-21T12:48:40Z

I have tested the patch, but i can´t use dd without dl.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4406.}

nternetinspired · 2015-08-23T08:47:46Z

Tested the patch on serveral templates and got no differnece in the appearance. (not patched/patched)
In some templates the html-code got some diffrent white spaces and line breaks but the essential code was the same.

There isn't supposed to be a difference in appearance @G-Lu .

I have tested the patch, but i can´t use dd without dl.

Correct. What is your point @Blutengel89 ?

hans2103 · 2015-10-10T11:00:24Z

I have tested this item ✅ successfully on c76a74e

Great improvement!
Moving HTML to the correct place is well executed.
Before & after patch the HTML output looks the same.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4406.}

slibbe · 2015-10-10T11:18:12Z

I have tested this item ✅ successfully on c76a74e

I have tested this succesfully with no apparent diffence.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4406.}

nternetinspired · 2015-10-10T14:43:41Z

Thanks for testing @hans2103 and @slibbe :)

zero-24 · 2015-10-10T18:59:50Z

RTC based on tests. Thanks

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4406.}

Hackwar · 2015-10-11T12:34:12Z

This change is not backwards compatible. If you overrode block.php, you will end up with missing dd-tags around your code. I also still would rather see this in one big file than in a gazillion tiny ones. There is NO gain at all by putting every single line of code in its own file, except that we unnecessarily inflate the codebase with comments and file headers. This also is becoming a performance problem.

We can not improve this in any way in 3.x, since moving it all in one big file would prevent overrides for one of the small files to work and changing the code in the files like in here breaks existing overrides.

Might be a good reason to attack 4.0 without trying to invent the great solution for everything. Keep 4.0 simple by simply droping the deprecated stuff from 3.x and fixing stuff like this one and then release that.

zero-24 · 2015-10-11T13:11:12Z

Pending

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4406.}

roland-d · 2015-10-31T21:51:25Z

@nternetinspired Thank you for your contribution. The PLT has decided to close this PR and not merge it in Joomla 3 but reconsider it for Joomla 4. We want to look at not scattering a page around in so many single files.

Block party

eb08492

Signed-off-by: Seth Warburton <seth@internet-inspired.com>

jissues-bot added the PR-staging label Sep 30, 2014

Die whitespace!

c76a74e

Signed-off-by: Seth Warburton <seth@internet-inspired.com>

810 mentioned this pull request Oct 13, 2014

Info_block render helper not hiding all html #2447

Closed

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 10, 2015

zero-24 added this to the Joomla! 3.5.0 milestone Oct 10, 2015

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 11, 2015

roland-d closed this Oct 31, 2015

Bakual added the Re-evaluate for v4.0 label Oct 31, 2015

Bakual removed this from the Joomla! 3.5.0 milestone Oct 31, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block party #4406

Block party #4406

nternetinspired commented Sep 30, 2014

brianteeman commented Sep 30, 2014

block is changed.

nternetinspired commented Sep 30, 2014

wilsonge commented Oct 1, 2014

nternetinspired commented Oct 1, 2014

Bakual commented Oct 1, 2014

nternetinspired commented Oct 1, 2014

nternetinspired commented Oct 1, 2014

infograf768 commented Oct 2, 2014

nternetinspired commented Oct 2, 2014

Hackwar commented Nov 17, 2014

nternetinspired commented Nov 17, 2014

G-Lu commented Aug 21, 2015

Blutengel89 commented Aug 21, 2015

nternetinspired commented Aug 23, 2015

hans2103 commented Oct 10, 2015

slibbe commented Oct 10, 2015

nternetinspired commented Oct 10, 2015

zero-24 commented Oct 10, 2015

Hackwar commented Oct 11, 2015

zero-24 commented Oct 11, 2015

roland-d commented Oct 31, 2015

Block party #4406

Block party #4406

Conversation

nternetinspired commented Sep 30, 2014

A premise, I promise

Paints go in a paintbox, pencils in a pencil tin.

Once upon a time…

TL:DR;

Testing

brianteeman commented Sep 30, 2014

block is changed.

nternetinspired commented Sep 30, 2014

wilsonge commented Oct 1, 2014

nternetinspired commented Oct 1, 2014

Bakual commented Oct 1, 2014

nternetinspired commented Oct 1, 2014

nternetinspired commented Oct 1, 2014

infograf768 commented Oct 2, 2014

nternetinspired commented Oct 2, 2014

Hackwar commented Nov 17, 2014

nternetinspired commented Nov 17, 2014

G-Lu commented Aug 21, 2015

Blutengel89 commented Aug 21, 2015

nternetinspired commented Aug 23, 2015

hans2103 commented Oct 10, 2015

slibbe commented Oct 10, 2015

nternetinspired commented Oct 10, 2015

zero-24 commented Oct 10, 2015

Hackwar commented Oct 11, 2015

zero-24 commented Oct 11, 2015

roland-d commented Oct 31, 2015