Simple theme fixes#2976
Conversation
In the simple theme, some templates are using `h1`, others are using `h2` for the main title of the page (other than the one in the header). This commit changes that so all of the pages are using `h1`.
Add a <main> tag to surround all the content blocks, so that it's easier to target the main part of a page (be it an article, the list of posts or the different categories) using CSS. Because of this, the <section> part of the article.html template is made redundant, so it is removed. Finally, the generic <div> is replaced by an <article> tag to surround the article's content.
|
LGTM. |
|
I think it's already using a lot of HTML5 tags ( One comment I have is that the templates are using a lot of <body id="index" class="home">
<header id="banner" class="body">
(...)
<footer id="contentinfo" class="body">I don't really see the point of the |
|
I agree with your perplexity about the presence of a lot of |
|
So do you think I should rework this as well? I can trim a lot of things, since a lot of tags can be targeted by a specific CSS rule without having to rely on a specific |
|
Yes, from my point of view, but I think it would be better to seek the advice of other core developers |
|
OK, then I'll wait for others to comment on this and take action, if any :) |
|
I went ahead and made some modifications to the |
|
LGTM |
All of the modified HTML tags can be accessed in CSS without the need for a dedicated id or an additional class.
pieqq
force-pushed
the
simple-theme-fixes
branch
from
ab58bdc to
16b8a03
Compare
|
I've rebased this branch onto master. Not sure if this can be merged now? |
| <section id="content" class="body"> | ||
| <header> | ||
| <h2 class="entry-title"> | ||
| <h1 class="entry-title"> |
There was a problem hiding this comment.
If we're moving simple theme to a classless theme we can remove also this class and all the others.
There was a problem hiding this comment.
I decided to leave this class because in the base.html also has an <h1> tag within a <header> tag, so I thought it would still be easier to target the entry title using a dedicated class in CSS. What do you think?
|
Thanks for the enhancements and your patience, Pierre. Much appreciated! Kudos also to @pauloxnet for the help with reviewing. 👏 |
| </header><!-- /#banner --> | ||
| <nav id="menu"><ul> | ||
| </header> | ||
| <nav><ul> |
There was a problem hiding this comment.
Changes like this one will break websites that extend the Simple theme; what was the motivation?
Many websites extend the Simple theme and add their own CSS; removing these will break them. Was anything gained in the removal? |
|
@cpitclaudel: Avoiding disruption for existing users is a priority for us. Rather than speaking in theoretical terms, could you point us to specific sites that have been affected by this change? |
4 participants
A set of changes for the
simpletheme to be mobile-friendly and to useh1consistently throughout the template files. Please check individual commit messages for more info.