User tests: Successful: Unsuccessful:
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.
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.
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.
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.
Apply patch and check that neither appearance or markup of article info block is changed.
Labels |
Added:
?
|
Category | ⇒ | Layout |
I personally can't see any good reason for doing it like that.
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
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.
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.
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.
I'm quite sure a kitten would die somewhere due to this
Agree, it would be pretty offensive to me also!
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
Trailing space removed.
Note: This PR also adds the closing lines that were not present in child layouts ;)
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.
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…
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.
I have tested the patch, but i can´t use dd
without dl
.
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 ?
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.
I have tested this item successfully on c76a74e
I have tested this succesfully with no apparent diffence.
Status | Pending | ⇒ | Ready to Commit |
RTC based on tests. Thanks
Labels |
Added:
?
|
Milestone |
Added: |
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.
Status | Ready to Commit | ⇒ | Pending |
Pending
Labels |
Removed:
?
|
@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.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-10-31 21:51:26 |
Closed_By | ⇒ | roland-d |
Labels |
Added:
?
|
Milestone |
Removed: |
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:
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/