Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML5 elements in layouts #795

Closed
ghost opened this issue Mar 3, 2015 · 12 comments · Fixed by backdrop/backdrop#767
Closed

HTML5 elements in layouts #795

ghost opened this issue Mar 3, 2015 · 12 comments · Fixed by backdrop/backdrop#767

Comments

@ghost
Copy link

ghost commented Mar 3, 2015

I didn't want to re-open #360 since it's already been committed, so thought I'd start a new discussion here instead. I'm no expert in HTML5, but thought I'd make some suggestions for core layouts (specifically the 'two-column' layout I'm using as a reference) based on some research I've just done.

Why do messages use a <section> element? According to html5doctor.com <section> elements should have a heading that describes the theme of the section. Messages have no such heading, and shouldn't have. They're just for conveying contextual information to the user and, in my option, aren't part of the 'content' of the page... I think a <div> will suffice here.

Can we remove <a id="main-content"></a> and instead add id="main-content" to <main class="l-content" role="main">? This will remove an otherwise unnecessary, empty element. Also, according to an answer on StackOverflow, HTML5 looks first for id="foo", then name="foo" as a secondary option (making the first more correct).

Can/should we wrap

<?php print render($title_prefix); ?>
<?php if ($title): ?>
  <h1 class="title" id="page-title">
    <?php print $title; ?>
  </h1>
<?php endif; ?>
<?php print render($title_suffix); ?>

in <header> tags? I think that would make sense. We could even add the tabs and action links into the header section...

My understanding of the revised <aside> element is that it can now be used for sidebars (see http://html5doctor.com/aside-revisited/). I recommend replacing <div class="l-sidebar"> with <aside class="l-sidebar">.

Again, according to html5doctor.com, I think we should replace <div class="l-footer"> with <footer class="l-footer">.

@ghost ghost added the needs - more feedback label Mar 3, 2015
@quicksketch
Copy link
Member

  • <section> element on messages: I don't think we rationalized this fully in the other issue. We mostly used <section> because previously we had <div class="section">. I think you're right it should be a <div>.

  • On removing <a id="main-content"></a>: I think this is primarily separated because it allows for more flexibility when styling. Some times if you have a sticky header, the only solution to keep anchor tags jumping to the right window scroll is to set a negative top margin on the target element.

  • On id vs. name attribute: We're already using an id here, so the comments about using a name attribute don't apply.

  • On <header> tags, 90% of the time, $title_prefix and $title_suffix are empty. They're only used to add in contextual links (e.g. edit view, configure block, etc.) I don't think we should wrap in a <header> when most visitors aren't going to see anything but the <h1>.

  • On <aside> for sidebars: The article you reference states it pretty clearly:

    The aside element can now represent secondary content when used outside of an article. Keep in mind that aside — and, more generally, secondary content — does not necessarily mean “sidebar”. The style of the content should not dictate the use of the element. For content that is not the primary focus of an article (or page) but is still related to the article (or page), aside is what you need, regardless of its visual design.

    We don't know what content will be in the sidebar. <aside> is for related, secondary content. Given current paradigms, the sidebar will probably contain things like navigation, advertisements, or other "unrelated" content. So <div> is more appropriate because it is merely a presentational wrapper. If you have content in the sidebar that is related to the main article content, then that individual block should probably be an <aside>, which you can do using the Dynamic Block option within layouts.

  • On <footer>: It looks like we had intended to use this tag for the footer but it was accidentally left out of the changes.

@ghost
Copy link
Author

ghost commented Mar 3, 2015

I don't think we should wrap in a <header> when most visitors aren't going to see anything but the <h1>.

The whole point of semantics is assigning meaning to things. Whether or not visitors are going to
see title pre/suffixes doesn't take away from the fact that they are a part of the title and hence a part of the heading. I think a <header> wrapper is appropriate.

Otherwise I agree with/accept everything else you said.

@quicksketch
Copy link
Member

Whether or not visitors are going to see title pre/suffixes doesn't take away from the fact that they are a part of the title and hence a part of the heading. I think a

wrapper is appropriate.

I don't think our themers are going to be happy with us adding a <header> tag around nothing but an <h1>. What about adding a <header> tag if a prefix or suffix is present? Though this logic would start to get a bit messy and would make moving the title into a block (#796) more important.

@ghost
Copy link
Author

ghost commented Mar 3, 2015

In my first post above I also suggested the possibility of adding the tabs and action links to the <header> (along with the title). This may make more sense to themers, but may affect the ability to make them all individual blocks as per #796...

@ghost
Copy link
Author

ghost commented Mar 3, 2015

Here are the 3 options as I see them:

  1. No changes:
<?php print render($title_prefix); ?>
<?php if ($title): ?>
  <h1 class="title" id="page-title"><?php print $title; ?></h1>
<?php endif; ?>
<?php print render($title_suffix); ?>

<?php if ($tabs): ?>
  <div class="tabs"><?php print $tabs; ?></div>
<?php endif; ?>

<?php print $action_links; ?>
<?php print $content['content']; ?>
  1. Add <header> to title:
<header>
  <?php print render($title_prefix); ?>
  <?php if ($title): ?>
    <h1 class="title" id="page-title"><?php print $title; ?></h1>
  <?php endif; ?>
  <?php print render($title_suffix); ?>
</header>

<?php if ($tabs): ?>
  <div class="tabs"><?php print $tabs; ?></div>
<?php endif; ?>

<?php print $action_links; ?>
<?php print $content['content']; ?>
  1. Add <header> to title, tabs and action links:
<header>
  <?php print render($title_prefix); ?>
  <?php if ($title): ?>
    <h1 class="title" id="page-title"><?php print $title; ?></h1>
  <?php endif; ?>
  <?php print render($title_suffix); ?>

  <?php if ($tabs): ?>
    <div class="tabs"><?php print $tabs; ?></div>
  <?php endif; ?>

  <?php print $action_links; ?>
</header>

<?php print $content['content']; ?>

@quicksketch
Copy link
Member

I think the 3rd option makes a lot of sense. We should probably add a class on there as well don't you think?

If we did this approach, all of those items would also likely be grouped into a single block in #796. We'd probably provide a default template for such a block to remove all the wrappers, just like we do in block--system--main.tpl.php.

@ghost
Copy link
Author

ghost commented Mar 3, 2015

I agree :)

@quicksketch
Copy link
Member

Seven has some odd requirements around the positioning of its title/tabs/action links that makes wrapping them with a single <header> tag difficult. See #796 (comment).

@ghost
Copy link
Author

ghost commented Mar 4, 2015

I'd be happy to see them all as separate blocks. That would mean we don't need to worry about <header> as they're no longer in the .tpl.php file.

@ghost
Copy link
Author

ghost commented Mar 4, 2015

So this issue's now about changing the messages wrapper back to a <div> and changing the footer wrapper to <footer>.

@quicksketch
Copy link
Member

So this issue's now about changing the messages wrapper back to a <div> and changing the footer wrapper to <footer>.

Sounds good, we can make these changes in a bugfix release IMO. The change is minimal and theme's already should be dependent upon the region classes (if anything) rather than the tag name. This should be a trivial PR if you're up for it.

@ghost
Copy link
Author

ghost commented Mar 4, 2015

Here you go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant