Problem/Motivation
Default core markup (aka Stark) is messy. For example, theme_item_list() includes:
$output = '<div class="item-list">';
if (isset($title) && $title !== '') {
$output .= '<h3>' . $title . '</h3>';
}
which is arguably unnecessary and superfluous. Given that we are converting the entire core markup to Twig in the new theme system, this presents an opportunity to remove things that may not be necessary. Whether Twig makes it into D8 or not, we'd like to see a big clean-up in Stark, and improvement of Bartik.
We've been using the "theme system cleanup" tag to track things like this in general, but this issue could establish some directions and goals for the themes.
Proposed resolution
Goals for core themes: Make Stark as semantic as possible; Make Bartik a better prospective base theme
After some discussion at the 2012 BADCamp Twig Sprint we expressed a desire to have core default markup (i.e. what Stark provides) to be as barebones as possible. Thus, the target audience for using stark would be those themers who truly want to start with a clean slate, and don't wish to have to work backwards to reach that slate.
Accordingly, we'd like to see Bartik become a better prospect for a base theme so that it is easy to start with and build from.
For example, move some of the functionality from theme.inc to Bartik's template.php. Or have stark contain no regions (aside from content) by default.
An amazing inspirational video from mortendk.
http://www.youtube.com/embed/2LwMn9oHOx8
There are three major Drupal initiatives that change the markup:
- Mobile and HTML initiatives - CSS/Architecture clean up #1921610: [Meta] Architect our CSS
- Twig initiative - all template files in core are now .html.twig files
- Seven theme in D8 - Seven theme Sandbox
Remaining tasks
Needs change notice
Patches (please review / revise)
- #1982256: Clean up html.html.twig markup
- #1842140: Remove title and wrapper div from item-list.html.twig
- #2041793: install-page.html.twig markup and CSS
- #1982256: Clean up html.html.twig markup
- #1982214: Markup for: theme_breadcrumb()
- #2004252: node.html.twig template
- #1982230: Modernize markup for the locale module
- #1982244: Markup for: block module
- #1982266: Markup for: link module
TODO (please write patches)
- #1912536: Remove all references to the (removed) user-picture.html.twig file Assigned to: vollepeer
- #1982202: Markup for: theme_status_messages()
- #1982220: Markup for: theme_progress_bar()
- #1912516: Markup for menus
- #1912608: Update pagination markup for new CSS standards and improved accessibility
- #1982200: Markup for: custom_block module
- #1189806: Convert aggregator-item.html.twig to HTML5
Discuss (please decide what todo)
- #2099289: Do we need a div around a more link?
- #2031883: Markup for: comment.html.twig
- #1982208: Replace indentation theme hook/indentation.html.twig with data attributes
- #1982236: Markup for: theme_feed_icon()
- #1982240: Markup for: book module
- #1982248: Markup for: views/templates/views-view-list.tpl.php
- #1982270: Markup for: overlay module
- #2177813: details.html.twig should include .description class
User interface changes
None, at the moment. Regions may disappear in Stark.
API changes
TBD
Related Issues
#1843798: [meta] Refactor Render API to be OO
#1804614: [meta] Consolidate theme functions and properly use theme suggestions in core
#1804488: [meta] Introduce a Theme Component Library
#1499460: [meta] New theme system
#1087784: Add new theme to Drupal 8.1.x or another minor version
#1854344: [meta] Goals for core themes: Make Stark as semantic as possible; Make Bartik a better prospective base theme
#1854672: remove seven_node_add_list() from core (update the markup in theme_node_add_list() instead)
#1842140: Remove title and wrapper div from item-list.html.twig
#912458: Design Initiative: Core theme selection and development process
#737136: Put together a list of must-have features for new core themes and set a final deadline for implementing them
#dreammarkup
One of the goals of the Twig initiative is to improve the markup in core. Many contributors are still involved continuing to converting templates to Twig, and now for the completed templates we can begin to improve the markup according to following principles.
Markup principles
Class names
Class name convention is based on CSS architecture (for Drupal 8) documentation. One of the pitfalls of current CSS is that we have something like .menu li a
but for the new convention is should be .menu__link
. This will reduce the weight of the selector and will make themes cleaner.
The id attribute
It is recommended to avoid the id attribute for styling purposes. Classes should be used instead to apply styles to an element. However, the id attribute is needed for other purposes. In general, avoid removing ids from the markup itself unless you are sure they were only used for CSS.
Markup
Keep in mind that one of our principles is to start from nothing. That means using as few HTML elements and attributes as possible in core. If Bartik needs a div around something for its own purposes, that div tag should be added in the Bartik theme, not in core.
When re-writing markup try to be aware of the repercussions on other code, but don’t let them stop you. See the action steps below for how to deal with changes that affect existing CSS, JS, etc.
Action steps
1. Find a file.
Either choose one of the completed Twig templates (from the list above) that you wish to improve. Search for existing issues tagged "dreammarkup" that pertain to the template file.
2. File an issue (if none exist).
Add your issue to Twig sandbox. In issue summary provide the path to the file, link to this meta issue and a code proposal ( optionally with a patch ) for a better markup. Add dreammarkup
tag. Also update this issue summary above with your issue id.
3. Add link to your issue here under 'Issue list' section.
In addition to the tag, this is a good way for us to monitor progress via this meta issue.
4. Write and post a patch.
Mark your issue needs review if you have a patch or idea, or needs work if you need to work on it or it is in progress. Make sure to justify your changes.
5. Check for related CSS and Markup changes.
If (and probably because) markup changes can break all the CSS - check files in which this markup is used, and if possible provide patch for CSS fix specific for your changes. Do not fix spaces or tabs - that is the job of CSS cleanup initiative: #1921610: [Meta] Architect our CSS
Comments
Comment #0.0
oresh CreditAttribution: oresh commentedlinks fixed.
Comment #0.1
oresh CreditAttribution: oresh commentedSteps added
Comment #0.2
oresh CreditAttribution: oresh commentedmarkup change
Comment #0.3
oresh CreditAttribution: oresh commentedcode fix.
Comment #1
c4rl CreditAttribution: c4rl commentedTagging
Comment #1.0
c4rl CreditAttribution: c4rl commentedTwig templates section added
Comment #1.1
oresh CreditAttribution: oresh commentedtwig rtbc link added.
Comment #1.2
oresh CreditAttribution: oresh commentedTwig templates meta issue link added
Comment #1.3
oresh CreditAttribution: oresh commentedmarkup changed :) it's a markup issue.
Comment #1.4
c4rl CreditAttribution: c4rl commentedFormatting
Comment #1.5
c4rl CreditAttribution: c4rl commentedClarify
Comment #1.6
oresh CreditAttribution: oresh commentedIssue list items added.
Comment #1.7
oresh CreditAttribution: oresh commentedmarkup fix.
Comment #1.8
jenlamptonexpand
Comment #2
ry5n CreditAttribution: ry5n commentedThe CSS coding standards advises never using id as a CSS hook. The performance difference between id and class is basically nonexistent, and selector performance in general is near the bottom of the list of bang-for-buck performance optimisations. I’d like to stick to the new CSS standards and use single classes as much as possible.
The example given of the new selector style is not great:
.menu__list-item__link
. A better replacement for the original selector.menu li a
would be.menu__link
. I need to add some clarifications to the standards based on what I’ve learned in the sandbox, but my rule of thumb is to make a class name only as ’ugly’ as needed for clarity. Class names do not need to encode markup structure, they only need to name things unambiguously. In general, prefer .menu__link over .menu__item__link.I believe many of the hidden headings in use by Drupal are there for accessibility reasons, which means the heading tags supply import semantic information to screen readers. I would hesitate to change this pattern unless we get the OK from our accessibility team.
Comment #3
ry5n CreditAttribution: ry5n commentedBTW +100 to this issue. Thanks @oresh for creating it.
Comment #3.0
ry5n CreditAttribution: ry5n commenteditem list
Comment #4
oresh CreditAttribution: oresh commentedThanks ry5n! I've changed the description.
I was thinking that the class name was kinda ugly, but decided to stick to DOM structure. Your variant is cleaner and more readable.
I'll try to post an article about this issue to Drupal planet - hope to get some more attention to it.
While 'more back-end' front-end developers are involved in Twig initiative, we should try to make the Markup in parallel asap if we want it to be in core until the release.
Comment #4.0
oresh CreditAttribution: oresh commentedChanged class names.
Comment #5
ry5n CreditAttribution: ry5n commentedFantastic. Thanks @oresh. I’ll be working on this again too and giving it everything I’ve got.
Comment #6
falcon03 CreditAttribution: falcon03 commentedJust a quick note: please, do not remove any hidden content from the markup.
Anything wrapped in a div/span/heading which the "element-invisible" class is assigned to is there to improve Drupal pages acessibility. Unfortunately I am in a hurry right now, but I'll provide more detailed feedback as soon as possible.
Comment #7
ry5n CreditAttribution: ry5n commentedWhat @falcon3 said, but also not changing the tag either. Basically we should not touch that screen-reader markup except with a firm understanding of the a11y implications.
About the ‘start from nothing’ principle: this is really important. However, it applies most to core markup, i.e. the markup coming out of core modules and libraries, and of course the Stark theme, since it is designed to simply present raw core output. However, base themes (if ever in core) might want/need to provide additional markup/CSS/JS to fulfill their goals as something more convenient to build on. Finally, purpose-built core themes such as Seven will need to do the most building-up from the lean base markup envisioned for core modules.
My point is that not all core markup should be stripped down to ’nothing’. The more generic and extensible the markup needs to be, the more basic and clean. The more specific, the more it should simply be well-architected for that purpose.
Comment #7.0
ry5n CreditAttribution: ry5n commentedgrammatical error fix.
Comment #7.1
carwin CreditAttribution: carwin commentedFixed a minor typo.
Comment #7.2
oresh CreditAttribution: oresh commentedvideo
Comment #7.3
oresh CreditAttribution: oresh commentedvideo ifrmae
Comment #7.4
oresh CreditAttribution: oresh commentedimage instead of iframe
Comment #7.5
oresh CreditAttribution: oresh commentedohhh those images :(
Comment #7.6
c4rl CreditAttribution: c4rl commentedAttribution
Comment #7.7
oresh CreditAttribution: oresh commentednew list added.
Comment #7.8
oresh CreditAttribution: oresh commentedAdded new issues.
Comment #7.9
oresh CreditAttribution: oresh commentedsteps improved.
Comment #7.10
oresh CreditAttribution: oresh commentedlink fixed.
Comment #8
barraponto CreditAttribution: barraponto commentedUpon visitting #1843774: Convert views/views_ui/templates/views-ui-display-tab-setting.tpl.php to Twig I noticed the usage of non-breaking space without any explicit reason for it. Grepping for nbsp showed up quite a few uses, including form.inc and theme.inc (not to mention js plugins and vendored PHP). Shouldn't we think about nbsp usage? What is the rationale to add it or avoid it?
Comment #9
barraponto CreditAttribution: barraponto commentedOn the invisible headers topic: it builds the document layout which is quite important for accessibility and semantics. See http://www.netmagazine.com/features/truth-about-structuring-html5-page for more on it.
Also, when it comes to clashing element ids, we can always use drupal_html_id to avoid the clashes... althought I'm not sure what's the use of an id attribute if you can't rely on its value. Maybe for javascript behaviors, passing the generated id as a setting -- although I didn't see that pattern around.
Comment #9.0
barraponto CreditAttribution: barraponto commenteduser name fail
Comment #10
oresh CreditAttribution: oresh commented@barraponto, thanks for the review. the article is just amazing.
I've removed the 'header' section, cause I don't find it necessary at all now.
About the ID's - they are used by JavaScript, and sometimes by CSS (yep, sometimes it is).
Secondly i like to keep the representation of unique elements on the page by their id attribute, so you can define - 'ok, this element should only be used once', or something like that. Anyway leaving or deleting ID is not critic for most of the time, so we provide them by default, but themers decide themselves to use or not the id :)
Comment #10.0
oresh CreditAttribution: oresh commentedremoving header section
Comment #10.1
ry5n CreditAttribution: ry5n commentedUpdate instruction sections for id and markup to clarify stance on ids (their purpose, when to remove, recommendation to not use in CSS). Corrected some punctuation and typos.
Comment #10.2
scor CreditAttribution: scor commentedadd link to node markup
Comment #11
LewisNymanWe discussed our approach for each one of these issues. We think we can commit each patch separately, without sandboxing.
Comment #12
c4rl CreditAttribution: c4rl commentedI created #1854344: [meta] Goals for core themes: Make Stark as semantic as possible; Make Bartik a better prospective base theme before we started talking about dreammarkup; it's possible that it may be a duplicate and we should combine these. Thoughts?
Comment #12.0
c4rl CreditAttribution: c4rl commentedwrong issue # for node.html.twig
Comment #13
ry5n CreditAttribution: ry5n commented@c4rl We should probably merge them. I think #1854344: [meta] Goals for core themes: Make Stark as semantic as possible; Make Bartik a better prospective base theme has a really good overview of the goals. This issue is more focused on the particulars of implementation. Should this one be made a sub-issue of that? Or should Dream Markup be a sub-section in 1854344?
Comment #13.0
ry5n CreditAttribution: ry5n commentedadd related issues
Comment #13.1
mortendk CreditAttribution: mortendk commentedadded link to page.html.twig
Comment #14
killtheliterate CreditAttribution: killtheliterate commented@c4rl these issues look to like duplicates to me. I think combining them makes sense.
Comment #14.0
killtheliterate CreditAttribution: killtheliterate commentedRemove link to Twig sandbox now that the templates are in core :)
Comment #14.1
jenlamptonadded text from https://drupal.org/node/1854344
Comment #14.2
jenlamptonrelated issues too
Comment #15
jenlamptonSince there are already a bunch of people following this issue, I closed #1854344: [meta] Goals for core themes: Make Stark as semantic as possible; Make Bartik a better prospective base theme as dupe.
I did also move over the summary, so everything we need should be here now. :)
Comment #15.0
jenlamptonadd title
Comment #15.1
jenlamptoncomment
Comment #15.2
jenlamptonmeter, ha
Comment #15.3
star-szrRemove dupe breadcrumb issue
Comment #15.4
mortendk CreditAttribution: mortendk commentedadded in links to proposed patches
Comment #15.5
mortendk CreditAttribution: mortendk commentedadded more issues
Comment #15.6
jenlamptonup
Comment #15.7
jenlamptonup
Comment #15.8
jenlamptonup
Comment #15.9
jenlamptonup
Comment #15.10
jenlamptonupdate
Comment #15.11
jenlamptonup
Comment #15.12
jenlamptonmore
Comment #15.13
jenlamptonmore link
Comment #15.14
jenlamptondedupe
Comment #15.15
jenlamptonup
Comment #15.16
joelpittetUpdated issue summary.
Comment #16
John Pitcairn CreditAttribution: John Pitcairn commentedAdded an issue for Details markup: #2177813: details.html.twig should include .description class
Comment #17
LewisNymanComment #18
LewisNymanThis needs an issue summary update for a post-banana world. Or we could just close it.
Comment #19
mortendk CreditAttribution: mortendk commentedi think we should close it - The banana fixed all the things.
Hopefully we can also stop the silly idea's that theres a one way of doing markup
Comment #20
sqndr CreditAttribution: sqndr commentedThere hasn't been any work in the last year. As mentioned by Lewis and Morten, we should close this issue. Banana fixes "all the things".
Marking as duplicate to clean up the issue queue.
Comment #21
lauriiiOpening this since there is still open child issues.
Comment #22
joelpittet