Is it a problem that this PR will change the H tags on existing sites?

On 19 June 2015 at 11:51, Viktor Vogel notifications@github.com wrote:

Sets the correct heading tag for the title in the article view if no page
heading is provided + code styles

At the moment the h1 tag is used for the page heading and h2 tag for the
title in the article view of the content component. But if no page heading
is provided, the title still has only the h2 tag and not the h1 tag as it
should be.

I also couldn't resist to clean the file a little bit and improve the code
style. So, the main change for the fix is in line 42 and line 46.

How to test

1. Create an article
2. Create an link to this article in the menu
3. In the menu entry go to the tab "Page display" and enable the option "Show Page Heading". You should als enter a value in the option "Page Heading" or "Browser Page Title" to see the output in the frontend.
4. Open the article in the frontend with the link from the menu. You will see that the page heading will have an h1 tag and the article title an h2 tag.
5. Now go to the menu entry in the backend and disable the option "Show Page Heading".
6. Reload the article in the frontend and you will see that the page heading is not displayed any more but the article title still has the h2 tag instead of the correct h1 tag.
7. Apply this PR and check again. The title will have the h1 tag.

---

You can view, comment on, or merge this pull request online at:

#7203
Commit Summary

• Set the correct heading tag for title in the article view if no page heading is provided + code styles
• code style

File Changes

• M components/com_content/views/article/tmpl/default.php https://github.com/joomla/joomla-cms/pull/7203/files#diff-0 (281)

Patch Links:

• https://github.com/joomla/joomla-cms/pull/7203.patch
• https://github.com/joomla/joomla-cms/pull/7203.diff

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

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

@brianteeman Not a problem but an improvement! :-)

Tested on current Version 3.4.1 enviroment with php 5.5.26
Patch works correct without problems.

You're using spaces for indents...

@Viktor: In the article I use h3 as next lower title. Is there a new Problem if the title is h1 and the next lower title is h3?

@mbabker Good catch, thanks. Did it on my Mac, still have to optimize some settings there...

@Worti2 Don't think that this is a big problem. Of course you could then update your content and use h2 instead.

Sorry to say that (I really hate, as I'm not so much integralist as far as B/C goes), but isn't this a royal breach of B/C? A lot of sites are potentially already styled expecting the old behavior...

@smz Don't see a problem with B/C here. All overrides will still work the same as we don't introduce new variables. Yes, only users without any template overrides will benefit from this change. To make template creator aware of it, we could tag it as a new feature. What do you think?

@Kubik-Rubik, agreed that there will be no problem if your template uses overrides.

In my templates (I always roll-up my own templates) I normally tend not to use overrides, as much as I can, and I instead rely on a template.css stylesheet for most personalizations. I also tend not to use the "Page Heading" field, as much as I can.

Heading for my articles, thus, is displayed through a css declaration of this kind:

.page-header h2 {whatever-I-like-inside-this}

I might also have a very different style to be used when I want (i.e. the rare times I set the "Page Heading" explicitly for displaying something differently):

.page-header h1 {something-very-different-in-here}

With your PR applied, styling of my articles pages will change, and that why I'm calling this a breach of B/C

I agree that your solution is more semantically correct, but anyway...

If anyway we want to go on with this (I will not protest, I can fix my stylesheets...), I suggest that we take the occasion of this minor styling B/C breach so that headings deriving from menu-items ("Page Heading") and headings deriving from content title are embedded in two different classes.

Now both are in a .page-header div: we could use .content-header for headings coming from the "Title" attribute...

... also, when we do not display the content "Title" (Options, Display Title, No) we still generate a <div class="page-header"> with an empty <h2> (or <h1>) tag. Is there any reason for that? This causes issues if for example you are styling div.pag-header with a lower visible border. You wouldn't expect it being there if you don't display the Title...

The problem with this patch is that it has or can have a visual effect, the site looks different after an update. Fixing this is not a problem for an experienced user and I agree from the semantic it is better, but I wouldn't merge it.

@Robert: You are right with the not so expreienced users. And I try to represent them. But in my opinion this is an issue. Maybe we will change this in Joomla! 4 together with other template changes.

This break`s existing sites
Imagine a site with the following css

h1 {font-family:serif;size:32px;color:black;}
h2 {font-family:sans;size:16px;color:red;)

This PR completely changes the styling of the site

I think though indeed this might change existing site's look and feel somewhat (h1's and h2's are usually not that far away from eachother in pixel-size anyway) it does solve an ongoing issue with Joomla's H1-H2 structure. Simply because it was always an issue causing people to fix this in funny ways should not mean that we should stick to those funny solutions.
Also: if people now have an H2-H3 structure it won't be that bad if they then have a H1-H3 structure.

Just my 2 cents ;)

(h1's and h2's are usually not that far away from eachother in pixel-size anyway)

You have no way of knowing that and as my example shows it could be way more than pixel size

Correct, I am speaking in general, there may indeed some for which the difference is more visual.

I agree with Brian and Robert here.
I think it's something to consider for 4.0, but not worth the risk of changing the appearance of many sites.
Also this would have to be done in all other places as well, so it's consistent.

Okay, since we could produce a B/C problem with this change, I will close this PR.

Who wants to benefit already in the 3.x version, can use this patch: https://github.com/joomla/joomla-cms/pull/7203.patch

@Kubik-Rubik i have add the Joomla 4.0 label to re-evaluate this for v4.0 Thanks