? ? Success

User tests: Successful: Unsuccessful:

avatar nternetinspired
nternetinspired
30 Sep 2014

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.

:hocho: :dragon:

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.

avatar nternetinspired nternetinspired - open - 30 Sep 2014
avatar jissues-bot jissues-bot - change - 30 Sep 2014
Labels Added: ?
avatar brianteeman
brianteeman - comment - 30 Sep 2014

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: :hocho:][image: :dragon:]
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

Patch Links:


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

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

avatar brianteeman brianteeman - change - 30 Sep 2014
Category Layout
avatar nternetinspired
nternetinspired - comment - 30 Sep 2014

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

avatar wilsonge
wilsonge - comment - 1 Oct 2014

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

avatar nternetinspired
nternetinspired - comment - 1 Oct 2014

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.

avatar Bakual
Bakual - comment - 1 Oct 2014

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 :smile:

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.

avatar nternetinspired
nternetinspired - comment - 1 Oct 2014

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.

avatar nternetinspired
nternetinspired - comment - 1 Oct 2014

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

Agree, it would be pretty offensive to me also!

avatar infograf768
infograf768 - comment - 2 Oct 2014

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

avatar nternetinspired
nternetinspired - comment - 2 Oct 2014

Trailing space removed.

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

avatar Hackwar
Hackwar - comment - 17 Nov 2014

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.

avatar nternetinspired
nternetinspired - comment - 17 Nov 2014

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…

avatar Blutengel89 Blutengel89 - test_item - 21 Aug 2015 - Tested unsuccessfully
avatar Blutengel89 Blutengel89 - test_item - 21 Aug 2015 - Not tested
avatar Blutengel89 Blutengel89 - test_item - 21 Aug 2015 - Tested unsuccessfully
avatar G-Lu G-Lu - test_item - 21 Aug 2015 - Tested successfully
avatar G-Lu
G-Lu - comment - 21 Aug 2015

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.

avatar Blutengel89
Blutengel89 - comment - 21 Aug 2015

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.

avatar roland-d roland-d - alter_testresult - 21 Aug 2015 - G-Lu: Tested unsuccessfully
avatar nternetinspired
nternetinspired - comment - 23 Aug 2015

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 ?

avatar hans2103 hans2103 - test_item - 10 Oct 2015 - Tested successfully
avatar hans2103
hans2103 - comment - 10 Oct 2015

I have tested this item :white_check_mark: 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.

avatar slibbe slibbe - test_item - 10 Oct 2015 - Tested successfully
avatar slibbe
slibbe - comment - 10 Oct 2015

I have tested this item :white_check_mark: 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.

avatar nternetinspired
nternetinspired - comment - 10 Oct 2015

Thanks for testing @hans2103 and @slibbe :)

avatar zero-24 zero-24 - change - 10 Oct 2015
Status Pending Ready to Commit
avatar zero-24
zero-24 - comment - 10 Oct 2015

RTC based on tests. Thanks


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

avatar joomla-cms-bot joomla-cms-bot - change - 10 Oct 2015
Labels Added: ?
avatar zero-24 zero-24 - change - 10 Oct 2015
Milestone Added:
avatar Hackwar
Hackwar - comment - 11 Oct 2015

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.

avatar zero-24 zero-24 - change - 11 Oct 2015
Status Ready to Commit Pending
avatar zero-24
zero-24 - comment - 11 Oct 2015

Pending


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

avatar joomla-cms-bot joomla-cms-bot - change - 11 Oct 2015
Labels Removed: ?
avatar roland-d
roland-d - comment - 31 Oct 2015

@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.

avatar roland-d roland-d - change - 31 Oct 2015
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-10-31 21:51:26
Closed_By roland-d
avatar roland-d roland-d - close - 31 Oct 2015
avatar Bakual Bakual - close - 31 Oct 2015
avatar roland-d roland-d - close - 31 Oct 2015
avatar Bakual Bakual - change - 31 Oct 2015
Labels Added: ?
avatar Bakual Bakual - change - 31 Oct 2015
Milestone Removed:

Add a Comment

Login with GitHub to post a comment