? ? Success

User tests: Successful: Unsuccessful:

avatar n9iels
n9iels
8 Jul 2015

What this PR does

This PR add a parameter to hide of show the title of the Article info block. I'm talking about the title Details above the written by, category etc.
I made this PR because it was requested in the code, and also it seems a useful feature.

How to test this PR

1) Go to an article on the frontend-end and make sure the info block is enabled
2) Enable this PR
3) Confirm on the following places the parameter Show Article info title is visible and works:

  • Article options
  • Single article menu item options
  • Category Blog menu items options
  • Article manager configuration

4) Also confirm this parameter works with various combinations of parameters. Like Use default in the article configurtion and Show in the singel articl eoptions.

If I forgot a please where this parameter also can be use, please say so :)

avatar n9iels n9iels - open - 8 Jul 2015
avatar n9iels n9iels - change - 8 Jul 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 8 Jul 2015
Labels Added: ? ?
avatar zero-24 zero-24 - change - 8 Jul 2015
Category Front End UI/UX
avatar zero-24 zero-24 - change - 8 Jul 2015
Easy No Yes
avatar zero-24 zero-24 - change - 8 Jul 2015
Category Front End UI/UX Front End Layout UI/UX
avatar n9iels n9iels - change - 8 Jul 2015
The description was changed
avatar Bakual
Bakual - comment - 8 Jul 2015

If you're going to do that, you need to add default value "1" to the XML fields and also leave the default "1" in the code where the param is requested.

Personally I'm not a huge fan of this.

To me it doesn't sound like something you want to have enabled in one article and disabled in another one. And thus it would be very simple to do in a template override for that specific JLayout. Or even more simple just hide it using CSS, which has the advantage that screen readers would still have the title.

This PR would also create an empty <dt> tag if the title is hidden, which is not ideal either.

I think it would be better to just remove that check and the associated TODO.

avatar mbabker
mbabker - comment - 8 Jul 2015

So move the if conditional to include rendering the dt...

Removing the check forces you to always display the title that's within the conditional and also forces a site implementor to create an override to remove it or use some excessively complex code (either PHP or CSS) to check if it should show up based on the view that's being rendered. At least a param here is consistent with every other displayable item in the layout and keeps it relatively easy to manage. Also remember here that while overrides are powerful, that's more effort a site maintainer has to put into the site (every layout override in theory must be reviewed each time the site is updated to ensure the right data is being displayed).

avatar brianteeman
brianteeman - comment - 8 Jul 2015

Pretty sure an empty or non existent DT tag followed by a DD tag is invalid
markup.
On 8 Jul 2015 8:29 pm, "Thomas Hunziker" notifications@github.com wrote:

If you're going to do that, you need to add default value "1" to the XML
fields and also leave the default "1" in the code where the param is
requested.

Personally I'm not a huge fan of this.

To me it doesn't sound like something you want to have enabled in one
article and disabled in another one. And thus it would be very simple to do
in a template override for that specific JLayout. Or even more simple just
hide it using CSS, which has the advantage that screen readers would still
have the title.

This PR would also create an empty

tag if the title is hidden, which
is not ideal either.

I think it would be better to just remove that check and the associated
TODO.


Reply to this email directly or view it on GitHub
#7379 (comment).

avatar mbabker
mbabker - comment - 8 Jul 2015
avatar Bakual
Bakual - comment - 8 Jul 2015

My main question is still if that is something you want to behave differently for different articles / menu items. I would assume that is something you change as part of the site design and it will be the same behavior for each article.
And then it's something which could be done very simple using an override or even better with some simple CSS (while still having valid HTML and happy screenreaders):

dt.article-info-term {
  display: none;
}
avatar n9iels
n9iels - comment - 8 Jul 2015

Okay, that change wasn't very hard.
Only about the XML files, the default value isn't set by other field in the same file with the same function...

And yes agree with that, simply display none with css does the same trick.

avatar mbabker
mbabker - comment - 8 Jul 2015

That CSS snippet implies you want to hide it everywhere. If I want it to display on one category but not another, I'm either resorting to a CSS rule per category (most templates have body classes with this data) or checks in PHP for hardcoded authorized IDs. The point is that's actually rather complex when every other thing in the layout can be managed via param.

Also as Brian pointed out removing it completely causes invalid HTML markup if there are trailing <dd> items, so that check would have to be rethought in its entirety.

avatar n9iels
n9iels - comment - 8 Jul 2015

@Bakual Uhm, wait. The parameter in the article itself isn't working now. Didn't work before. Can you help me with that?

avatar Bakual
Bakual - comment - 8 Jul 2015

That CSS snippet implies you want to hide it everywhere. If I want it to display on one category but not another, I'm either resorting to a CSS rule per category (most templates have body classes with this data) or checks in PHP for hardcoded authorized IDs. The point is that's actually rather complex when every other thing in the layout can be managed via param.

Yeah, I really wonder if that is a use case. Hiding the title in one category / menu item / article / whatever and showing it in another. If it is, then a parameter makes sense. If not, then the parameter makes no sense at all imho.

Only about the XML files, the default value isn't set by other field in the same file with the same function...

I see you have the default="1" in the global config xml. That's fine. In the other XML files, the defaults are indeed not really needed since it selects by default the first option (Use Global) anyway.

The parameter in the article itself isn't working now. Didn't work before. Can you help me with that?

I'm not sure what you mean.

avatar n9iels
n9iels - comment - 8 Jul 2015

When you open a single article, and set the parameter to "hide" this make no sense. The info block sty visible

avatar Bakual
Bakual - comment - 9 Jul 2015

It works fine for me.
Keep in mind that if you have a menu item pointing to a single article, the settings from the menu item will override the settings from that article.

avatar superknutsel
superknutsel - comment - 11 Jul 2015

@test Worked for me, tested in article, blog, menu and in the mix.


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

avatar superknutsel superknutsel - test_item - 11 Jul 2015 - Tested successfully
avatar genesisfan
genesisfan - comment - 11 Jul 2015

@test succesfully tested, on article, menu single article, menu category blog menu options and article manager in combination with various parameters settings.


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

avatar genesisfan genesisfan - test_item - 11 Jul 2015 - Tested successfully
avatar roland-d
roland-d - comment - 11 Jul 2015

I agree with @Bakual as to the use of this PR. What the use case for this request @n9iels ?

avatar n9iels
n9iels - comment - 21 Jul 2015

When you like to remove that tile, without making a override

avatar Bakual
Bakual - comment - 21 Jul 2015

When you like to remove that tile, without making a override

@n9iels That's clear so far, but by that argument one would have to make every single output snippet optional.
What is wrong with using an override for that? Or CSS?
I can only see the use for such a parameter if you want to show the title on one page, but hide it on another page. But I think that's very unlikely as it is a design question which is usually consistent across the full site.

avatar n9iels
n9iels - comment - 21 Jul 2015

@Bakual Agree with that. I liked this option, because I mostly hide this option for each website.
The problem with overrides is, you have to keep them uptodate. But the info block doesn't change offten.

So, if everybody agree with this, I'm closing this PR and made a new one for removing the line comment // TODO: implement info_block_show_title param to hide article info title

avatar roland-d
roland-d - comment - 22 Jul 2015

@n9iels Sounds like a plan.

avatar n9iels n9iels - change - 22 Jul 2015
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-07-22 12:07:49
Closed_By n9iels
avatar n9iels n9iels - close - 22 Jul 2015
avatar Bakual
Bakual - comment - 22 Jul 2015

Agreed

avatar n9iels n9iels - reference | 54e19d2 - 22 Jul 15
avatar n9iels n9iels - head_ref_deleted - 4 Aug 2015
avatar MasterofAstro
MasterofAstro - comment - 15 Mar 2022

The

tag in HTML stands for definition description and is used to denote the description or definition of an item in a description list.
https://elementtutorials.com/ref/dd.html
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/7379.

avatar drmenzelit
drmenzelit - comment - 15 Mar 2022

@MasterofAstro is there any particular reason why you are commenting on a pull request that was closed 7 years ago?

avatar sharkeshd
sharkeshd - comment - 14 Dec 2022
avatar sharkeshd
sharkeshd - comment - 14 Dec 2022

Spam

Add a Comment

Login with GitHub to post a comment