User tests: Successful: Unsuccessful:
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.
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:
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 :)
Status | New | ⇒ | Pending |
Labels |
Added:
?
?
|
Category | ⇒ | Front End UI/UX |
Easy | No | ⇒ | Yes |
Category | Front End UI/UX | ⇒ | Front End Layout UI/UX |
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).
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).
Ya that's true too - https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dd
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;
}
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.
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.
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.
When you open a single article, and set the parameter to "hide" this make no sense. The info block sty visible
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.
@test Worked for me, tested in article, blog, menu and in the mix.
@test succesfully tested, on article, menu single article, menu category blog menu options and article manager in combination with various parameters settings.
When you like to remove that tile, without making a override
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.
@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
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-07-22 12:07:49 |
Closed_By | ⇒ | n9iels |
Agreed
The
@MasterofAstro is there any particular reason why you are commenting on a pull request that was closed 7 years ago?
Spam
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.