Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
As per title, probably changes and discussion here is going to pend on usage of the new sectioning and grouping elements such as <header>, <hgroup> and <footer>
. Screenshot added.
Comment | File | Size | Author |
---|---|---|---|
#106 | Screen Shot 2014-11-16 at 5.44.51 PM.png | 343.52 KB | mgifford |
#105 | page_twig_template.jpg | 415.4 KB | sqndr |
#105 | interdiff.txt | 464 bytes | sqndr |
#105 | followup_convert-1077578-105.patch | 2.28 KB | sqndr |
#83 | 1077578-page-tpl-sidebar-83.patch | 1.61 KB | visabhishek |
Comments
Comment #1
dcrocks CreditAttribution: dcrocks commentedBesides the system module bartik, garland, and seven all have a page.tpl.php, as well as the overlay module having overlay.tpl.php, a page.tpl.php substitute. The latter probably needs a substitute issue but I'm assuming the 3 basic themes will be mirrored in 8.x. Should they each have their own issue?
Comment #2
JacineThere are surely differences between them, and so the versions will be different too, but I think they can be handled in one patch. We should concentrate on
modules/system/page.tpl.php
first and go from there.Comment #3
ruplI'll be attacking this at BADCamp. Rawk.
Comment #4
JacineComment #5
ruplPosting my progress so the BADCamp sprinters can pick it up if necessary ;) See you guys in a bit!
Comment #6
Jacinejust an FYI that @chrispomery is working on #1189822: Convert maintenance-page.html.twig to HTML5, which is related.
Comment #7
scor CreditAttribution: scor commenteddid you mean to take off the wrapper elements here? Two divs get replaced with one div/section? In other words, was there no reason to have two div elements before (one of them being the wrapper)?
Comment #8
rupl@scor as I understood it, many of the wrappers are in place to cater to older browsers like IE6. Since we're not supporting IE6 in D8, I dropped the redundant wrappers without consulting anyone :)
When I popped in to the summit, I heard everyone discussing this topic, but unfortunately I couldn't stick around. If someone down there could post the consensus then we'll know how to proceed for all of these tpl re-writes.
Comment #9
aquariumtap CreditAttribution: aquariumtap commentedLooks good and it applied for me. I saw #main-wrapper and #page-wrapper divs were removed. Catering to IE6 isn't necessary, and a theme like stark that inherits the system's page.tpl.php shouldn't have any excess div tags for imagined use cases.
Comment #10
JacineHey, so why do we need these nested sections? I think those can all go... Doesn't make much sense.
-19 days to next Drupal core point release.
Comment #11
zachattack CreditAttribution: zachattack commentedRupl,
This is sort of a rough first pass based on some ideas I have been having about how to re-craft the default page output. I think that Drupal template by default should not be so markup heavy and also if I had more time to talk to you about this, I think that leveraging the renderable array system introduced in 7 to generate the default appearance of things like site_slogan etc.. In short many of the default items should only be printed without needing to wrap them in tags like h1's and links.
Again, the reason I am making this suggestion is that the default output doesn't need to be so feature rich as developers and users will download other themes to expand upon the features of the site.
I know this is not a patch but I wanted to get you my feedback and thoughts in the most expeditious way I could.
http://pastie.org/private/knce02r79at0fixu0ssgea
Best from AUSTIN
Z
Comment #12
aquariumtap CreditAttribution: aquariumtap commentedIt might be easier to list out the differences between the patch in #5 and the pastie from @zachattack.
The zachattack version has:
<hgroup>
around title and slogan. Title and slogan no longer has ID selectors #site-name and #site-slogan.<h2>
.<aside>
tag instead of<sector>
for sidebars.<sector>
wraps the entire page. Does not use<sector>
to wrap regions.<figure>
.Personally, I think the patch as it exists is fine, but the first two bullet points seem useful.
The render() approach will probably have to happen in a larger, meta sort of way. There doesn't seem to be any consensus on whether this was a good idea in D7, or how it should be approached in D8. I'm very interested in that conversation as well :)
Comment #13
ruplOk. I have taken feedback from @zachattack, @aquariumtap, and the gang at BADCamp and came up with the following:
<section>
as a generic wrapper around the page.tpl.php markup after reading that sections should generally represent records in a database. Page template encapsulates much more, so<div>
it is.<aside>
for blocks, but Jacine provided a compelling argument that the region wrappers are generic, and might not always represent asides that are related to content. In a blog, yes, they do. But not always.<hgroup>
because Hixie is probably pulling it out of HTML5 (thx Jeff Burnz). If it stays in the spec, we can add it later.Patches and cream!
Comment #14
aquariumtap CreditAttribution: aquariumtap commentedGreat! And blocks could always get their
<aside>
wrappers from block.tpl.php at some future point.I looked over the patch and applied it. Marking RTBC.
Nice meeting everyone today!
Comment #15
chx CreditAttribution: chx commentedI have my doubts. Doesn't the shiv need to come first?
Comment #16
catchI'd like to see a couple more reviews before this goes in, as well as get the shiv question answered.
Comment #17
Jeff Burnz CreditAttribution: Jeff Burnz commentedGeneral obeservations and thoughts, for discussion mainly, I am tending to think we can do better with more time and iterate this.
header
andfooter
(banner and contentinfo respectively), I think we can do thisarticle
as the wrapper for columns should be a div, its a style hook (which possibly could be removed)<div id=""></div>
, for example sidebars and footer have wrappers, but header and highlighted do not- do we even need these wrappers, if you think so, what for?div
for the site slogan the best we can do? What aboutp
?div
wrapper for the site name the best we can do - I question if this borders on being a presentational tag in this context, because afaikt all its doing is providing a non-semantic block level element, can we just usestrong
anddisplay: block;
or should we usep
tag with strong inside?With regards to the shiv, its sort of a yepno type answer - if we are not using an html5 specific tag for styling purposes we are safe, if we do, then ideally we need the shiv. So, what I think is that "no shiv" should not hold up core template conversions until such time as we really need to start caring about design/style, which is not now IMO - we know once that shiv goes in that small style things like some default margin in page.tpl.php is going to be fine (its going to apply in < IE9), however for much bigger changes such as a responsive Bartik or Seven, then yes, we absolutely need the shiv - so that is the point at which it will start blocking. This is my opinion at least on that matter.
Comment #18
chx CreditAttribution: chx commentedI think screenshots (even just attached not even inlined) would go very far with the "more reviews". Especially from IE which ... few of us... have.
Comment #19
Jeff Burnz CreditAttribution: Jeff Burnz commentedYellow = header region
Pink = highlighted
gray = sidebars
The screenshot is in FF7.
I coloured in these particular regions to show the gutters are not accounted for in this patch, but which start originally did have due to the following CSS which now does nothing:
Layout wise this looks the same in IE, with element differences due to no reset in core or Stark.
Comment #20
ruplAgreed on taking more time for this. This template definitely needs more discussion and testing. Here's another round:
<header>
and<footer>
added. Great catch.<div>
wrappers for some regions and not others: The wrapper divs for the columns are the only element demarcating that particular piece of content, while the$header
is implicitly wrapped by<header>
. I can also see the value in explicitly wrapping all regions, so if anyone else has an opinion, please weigh in.<p>
. Good idea.<article>
with<div>
for content wrapper. Consensus between front-end people is to push semantics to node or block scope.Regarding shiv: This in no way affects Stark. It doesn't ship with any styles that hit these tags, plus we're fixing CSS in another issue. This should not block our HTML5 issues. I guess gutters fall into the "CSS later" issue too.
Regarding screenshots: hmm. Stark is stark. Any styles and rendering will come from another place in Drupal. There will always be rendering differences and if someone is actually running a website using Stark, it seems like the visual presentation of the website is a low priority. If consensus is that we really need screenshots to show these minimally-styled markup changes, what should we be showing in them?
Comment #21
adnasa CreditAttribution: adnasa commentedI still think that we need an id or class attribute even when adding a role attribute.
Comment #22
catchWhile css might be a different issue I'd prefer it if we didn't leave completely redundant css in core because things are going out of sync. If there's a guaranteed way that won't happen then all well and good.
Comment #23
JacineThere is some misunderstanding going on here. I thought we were talking about something else, so sorry about that. Let me try to clarify: CSS that we are directly affecting in any given patch needs to be updated along with the patch. We can't break stuff or leave in garbage code or we may never find it again.
Here's what is separate:
If people really want to see side by side screenshots where nothing changes, that can be arranged, but I worry that in most cases it's probably more likely to confuse people than to actually be helpful.
Hopefully that clarifies things?
Patch review:
Thanks! :)
Comment #24
JacineBTW, there's an issue to tackle the default display properties for these that is RTBC so obviously that part should not be handled in this patch: #1321678: Provide default styling for block-level HTML5 elements.
Comment #25
ruplARIA roles
banner
andcontentinfo
are Landmark ARIA roles which carry explicit meaning, so they are more appropriate thanheader
andfooter
, which aren't included in the allowed values for ARIA roles. I had kept the HTML IDs the same as the role if present, but I can see where that may be confusing. This patch has IDs matching the tags instead of roles.I also added the
heading
role to theh1
. I'm not entirely sure if this is correct, but I didn't see anything advising against roles on heading tags.Wrappers
I don't want to add wrappers back in just for
.section
, so for this patch I removed the CSS rule Jeff pointed out in #19. If someone feels strongly about having wrappers for those 10px margins please chime in.Ok here's another patch, this time just a straight diff:
Comment #26
ruplAdded the "navigation" role for
<nav>
, removed "heading" from<h1>
.Comment #27
mgiffordPatch looks great. @rupl thanks for adding ARIA. Agreed that the H1 is better without the "heading".
Comment #28
JacineHmm, so this is ready from a markup standpoint IMO. The only outstanding issue is that the way the regions are being printed is completely buggy, e.g. an empty
<div>
always prints for tabs, etc. However, that's technically a separate issue. There's been a lot of discussion on it so far, but mostly for a solution that works for D7 too, so it shouldn't hold this up. So, I'm going to go out on a limb and mark this RTBC. :)That issue is here: #953034: [meta] Themes improperly check renderable arrays when determining visibility for anyone interested.
Comment #29
JacineBTW, here are before and after screenshots.
Comment #30
catch#26: 1077578-page-tpl-26.patch queued for re-testing.
Comment #32
ruplRerolled following /core directory commit!
Comment #33
JacineThat was quick. Thanks @rupl. ;)
Comment #34
catchCommitted and pushed to 8.x, thanks!
Needs a change notification I think, or possibly adding to http://drupal.org/node/1328756, not sure.
Comment #35
aspilicious CreditAttribution: aspilicious commentedComment #36
aspilicious CreditAttribution: aspilicious commentedChanging tag
Comment #37
JacineHmm, do we really need a change notification for every template we touch? Isn't it assumed that we break themes from major version to major version? There's no functionality or variable name changes here, just markup. I think the fact that we have an HTML5 initiative implies that we are changing markup in these templates. The html.tpl.php was a different story because we did make significant changes to the attributes and stuff. So, I don't know, but it would be good to set some guidelines for this stuff.
Comment #38
Everett Zufelt CreditAttribution: Everett Zufelt commentedFrom #1183042-28: Regression: Add WAI-ARIA roles to Core blocks
"I believe role="main" should go in the page.tpl and maintenance-page.tpl because the page content's title is in those templates and is part of the "main" content."
With which I agree. Can we open a followup issue to make this small template change and then rol that into this change notification please?
Comment #39
Jacine@Everett Zufelt It's already there. See line #118 of page.tpl.php.
Comment #40
catch[#1328756] might well be enough and we can just let that get updated as time goes on. Marking back to fixed then :)
Comment #41
catchHaving said that did we send out a /core announcement for the html.tpl.php patch? That is quite big news and worth spamming people :)
Comment #42
Jacine@catch Yup: http://groups.drupal.org/node/187004 :)
Comment #43
mgifford@Jacine just wondering about the order here:
<div id="main" role="main" class="clearfix">
Is it documented that it's id, role & then class? Should it be? I just asked as I was just proposing it in a different order for a different patch.
It doesn't matter, other than that consistency makes it easier to read and more professional.
Comment #44
Jacine@mgifford We don't have any rules for order of attributes ATM. That'd be hard to do in some places, but in cases like this where we're hard-coding them, we could certainly have coding standards in place for that. If you created an issue for this already, just link it back here and tag it "coding standards."
Comment #45
catch@Jacine and I had seen that too, but it was so long ago!
Comment #47
Snugug CreditAttribution: Snugug commentedIdentical to patch in comment #32, but added a clearfix class to the tag, as it seemed to be the only item that lost a class from the nested
Comment #48
mgifford@Snugug - going to change the Status?
Comment #49
Snugug CreditAttribution: Snugug commentedUm, yah, about that… woops! Done.
Again, don't quite know if this is 100% needed, and I realize it's a bit of an old thread to be resing, but I thought it was strange as I was backporting for the HTML5 Theme
Comment #50
aspilicious CreditAttribution: aspilicious commentedthis patch is alrdy fixed and in core....
???
Comment #51
Snugug CreditAttribution: Snugug commentedYa know what. sorry, you're right. Was in D7 land yesterday and new to Core contrib so that's the whole patch instead of just the part I wanted to update. Cloning into D8 now and will reroll and make a new thread.
Comment #52
mjohnq3 CreditAttribution: mjohnq3 commentedAdd the HTML5 element aside to sidebars.
Several years ago the W3C revised the specification to allow aside to be used for secondary content: "...a section of a page that consists of content that is tangentially related to the content around the aside element, and which could be considered separate from that content. Such sections are often represented as sidebars in printed typography."
HTML5 Doctor explains the use more fully.
Several widely used contrib themes; Zen, Boilerplate, Aurora, Sasson, Boron, and, Basic, are already using aside for sidebars.
(The attached patch may fail the testbot but I'll deal with that when it does.)
Comment #53
Anonymous (not verified) CreditAttribution: Anonymous commentedSince there was already a patch committed for this issue, you should create a new issue that describes what you want to update and post the patch there. You can reference this issue from the new issue that you create.
Comment #54
mjohnq3 CreditAttribution: mjohnq3 commentedThe last time I tried that I was asked to just re-open the original issue. #1880202: Add ARIA landmarks for Bartik theme (con't)
Comment #55
mjohnq3 CreditAttribution: mjohnq3 commentedComment #56
mgiffordI don't know if we need a 2013 reference for
<aside>
but the article you referenced was from 2009. How's adoption going? We're reconsidering<main>
so just want to make sure that this still has good support.Also, the patch has to deal with the shiv's for backward compatibility, right?
Comment #57
mjohnq3 CreditAttribution: mjohnq3 commentedAll browsers, with the exception of IE 8 and lower, provide support for the aside element. http://caniuse.com/#feat=html5semantic
The html5shiv, including the one provided with D8, supports the aside element.
Comment #58
mgiffordWell, I tested the patch in simplytest.me and Opera, FF, Chrome & Safari in the Mac. It could probably due with testing in IE too, but It's a simple change and seems to be supported.
Comment #59
xjm#52: 1077578-page-tpl-sidebar-52.patch queued for re-testing.
Comment #60
xjmHmm, I'm not sure we want to put this in without testing IE 8/9. :)
Comment #61
mjohnq3 CreditAttribution: mjohnq3 commentedPatch in #52 causes no visible problems in IE8 or in IE9. See attached.
Comment #62
mgiffordBut does IE8 always look this bad https://drupal.org/files/aside-ie-8.jpg
Comment #63
mjohnq3 CreditAttribution: mjohnq3 commentedThis issue doesn't actually concern Bartik but since it's the default theme, what you see is what you get. IE8 doesn't support media queries so you always get the "mobile-first" layout; content stacked over the sidebars.
At one time it was determined that we wouldn't be adding add various polyfills to support media queries and related items (CSS3, etc.) in older browsers. I'm not sure if that's still the case as D8 now ships with things like Moderizr, html5.js, etc.
Comment #64
xjmYep, only a single-column layout is supported for IE8 in core themes:
http://drupal.org/node/1722502
Comment #65
mgiffordOk, then I'm putting this back to RTBC.
Comment #66
mgiffordTalking with @Lee Rowlands about WAI-ARIA roles, i came across this recent documentation about HTML5 & WAI-ARIA:
https://dvcs.w3.org/hg/aria-unofficial/raw-file/tip/index.html#what-does...
They recommend using
role="complementary"
with<aside>
as "the default semantics are not implemented across browsers, so the default implied role, state, property, or suggested semantics (if no ARIA default) may be used."Comment #67
mgiffordI think this is the missing interdiff.
Comment #68
larowlanAssuming bot is ok, last patch only adds additional aria landmark roles
Comment #69
webchickWait. I'm a little confused by #66. At the top of the patch, we're making things asides, but we're not adding the complementary role. Is that a bug? Or why do we do it in the footer and not the sidebar?
Comment #70
mgiffordThat was an oversight. Not sure how I missed core/modules/system/templates/page.tpl.php when building #66. Sorry.
Comment #71
star-szrNeeds a reroll now that page.tpl.php is page.html.twig.
Edit: Not sure what to do with the issue title here :/
Comment #72
mortendk CreditAttribution: mortendk commentedi have just looked at this reroll and it seems to not follow at the principles that we have agreed upon by keeping the markup to a minimumetc.
- The plan is to keep stark as clean as possible & build bartik to be a basetheme, but those princippels grew out of the Twig initiativ :)
https://drupal.org/node/2008464#principles
give bartik its own branch to make it a strong starter theme
i would suggest that bartik is getting its own issue & that we fix stark first. then build on top of that - Bartik needs an epic reroll both in its markup and css, so it can become a truly good starter theme.
aside for the sake of aside ?
I must admit that i dont agree with the use of asides as defaults sidebars, its ment for "related content but not as related to the main article on the page".
Are we assuming that everything (and always) in the sidebars is related to the article. that is a thing we honestly dont know, untill we have content.
i would argue that a login field, a feed from a nother site, banner adds etc isnt related to the article but to the overall site and therefor they should be default be a div (i cant belive i just argued for using a div ;) So by forcing on the sexy "new" aside tag, we are misinforming by default. If a user wants aside it should be easy for him/her to change it in the markup. Lets not make up assumptions about the data.
I have rolled a patch that takes starks page.twig + the maintainence-page.twig and moves the secondary menus in the preprocess to bartiks.theme
#2011578: Markup for Stark's page.html.twig + maintenance-page.html.twig dunno if that should be added into this issue or ?
Comment #73
mgiffordJust looking at the patch you provided in #2011578 and realized that you hadn't included
role="complementary"
.That's actually one of the more prominent pieces of the patch in #70 above.
The HTML that needs to be added to make web pages accessible is changing. Limitations in screen reader software & inevitable delays in user behaviors means that to meet the WCAG 2.0 AA guidelines with Drupal 8 there is going to be some redundancy.
With HTML5 & WAI-ARIA more semantic meaning is being passed through to the screen reader. Some of the elements that were added in Drupal 7 are needed because it was the only way for screen reader users to navigate between sections of a page.
There is some overlap between the semantic information provided by HTML5 & WAI-ARIA. There is at least one person who has argued that
<aside>
should have default implicit ARIA semantic of "complementary". Sadly that thread didn't have any public follow-up.So, I think everyone likes clean, uncluttered code. Unfortunately, that can present accessibility problems if it isn't carefully considered.
Comment #74
pwieck CreditAttribution: pwieck commentedChanged to twig files with an issue besides above comments:
The <aside> tag is used differently throughout patch and HEAD twig files. Was this intended?
Comment #75
pwieck CreditAttribution: pwieck commentedStatus change
Comment #76
pwieck CreditAttribution: pwieck commented#74 Passed and ready for review, testing, or rework.
Comment #77
mortendk CreditAttribution: mortendk commentedlooking at the principels which were trying to build the 2 base themes for D8 this patch do try to add more than neede into stark it still adds classes thats not needed into stark and dosnt add em to bartik.
see https://drupal.org/node/2008464#principles
Comment #78
leslieg CreditAttribution: leslieg commented70: 1077578-page-tpl-sidebar-70.patch queued for re-testing.
Comment #81
Jeff Burnz CreditAttribution: Jeff Burnz commentedWhy are certain regions special cased? This makes no sense, it's old school/IE5/pre-mobile/pre-html5 thinking.
#73, as per #72 it's not possible to assume content relationships at the region level in a generic template. It's merely speculation that content appearing in "sidebar" regions bear a complementary relationship to the main content. The idea that special casing two regions achieves anything meaningful is seriously doubtful and needs to be questioned.
Comment #82
mgiffordComment #83
visabhishek CreditAttribution: visabhishek commentedFor core/modules/system/templates/page.html.twig changes are already committed.
So except that re-roll are
Comment #84
Risse CreditAttribution: Risse commentedLast patch seemed to be missing the last quote in 'role="complementary"'
Comment #85
sqndr CreditAttribution: sqndr commentedLooks like a good patch!
Comment #86
Nikolay ShapovalovIs it good idea to change order of div and aside?
Because in all other places in page.html.twig first goes div and then aside.
Comment #87
Nikolay ShapovalovPatch with changed order of div and aside in #featured.
Comment #88
mortendk CreditAttribution: mortendk commentedMay i ask why there still used ID's here ?
Its badpractice & all that jazz
Besides of that this patch makes little sense afaik ? this is a bartik patch the page.twig.html is fixed, so bartik should follow that template
Comment #89
dcam CreditAttribution: dcam commentedUpdating in prep for DrupalCon Austin sprints. See http://www.hook42.com/blog/prepping-drupalcon-austin-sprints-sprint-lead....
Removed old tags. The Novice tag was added in #71 because the previous patch needed a reroll, which was done in #74.
Comment #90
mgifford@mortendk - I'm definitely not a themer but trying to help nudge this issue along.
Wanted to see if this was closer to what you were looking for with the HTML.
I used meld to compare it with core/system/templates/page.html.twig and this is what I came up with.
It's definitely more than just an HTML5 conversion, but it looks a bit more consistent.
It's going to totally mess up the CSS though. Figured I'd get clarification on the HTML5 first.
Comment #91
star-szrI think the worthwhile changes to Bartik might be adding the ARIA roles but not sure anything else will be beneficial. The consensus in the recent past (~2 years) seems to be that Bartik is not worth saving as far as markup cleanliness.
This either needs a re-title and issue summary update or (better IMO) to be moved to a new, focussed issue and this one finally closed out. This "followup" has gone on for over a year now with no commits (see #52 / #53 where I agree with @linclark that a new issue should have been created).
Comment #92
mgifford@Cottser - agreed. So let's deal with the bigger followup issues that @MortenDK mentioned here #2286371: [Followup] Clean Up Markup for Bartik
So back up to @zniki.ru's post we'll focus on adding in ARIA.
Wanted to check in about this switch of div/aside.
It's more standardized for sure.. Seems fine to me.
Can we get #87 RTBC?
Comment #93
NiklanHello there.
page.html.twig conains
But w3c says:
For now (beta1) main tag contains sidebars, which contains navigtaions, search forms etc.
Comment #94
mgiffordI think this is a better W3C reference, but it doesn't alter your comment in any way.
This patch doesn't alter the placement of the
<main>
tag. Although it could. Might be better to be a follow-up issue. This would allow us to focus on moving the page.tpl.php to HTML5.The patch needs a re-roll and we could look at whether we can clean this up at the same time.
Comment #95
rpayanmReroll from #87.
Comment #96
mgiffordNobody has addressed @mortendk point in #88 about use of ID's.
I have added this follow-up issue for the #2361047: [Followup] Move <main> in page.tpl.php to respect HTML5 content used in the
<main>
tag.I'm for getting this in now. This patch at this point mostly adds an ARIA role
role="complementary"
. This should insert the semantics into the page to clearly mark that as complementary content.Comment #97
mortendk CreditAttribution: mortendk commentedchange the issue name so we know its about bartik
Comment #98
Jeff Burnz CreditAttribution: Jeff Burnz commentedThe reason we used ID's in D7 Bartik was because we wanted to build a super-robust theme that would not break easily, no matter what CSS was thrown at it from contrib modules and the like. We felt it was more important to build something reliable and near enough to unbreakable since it was core and lots of sites were likely to use the default theme, as many in the past were using Garland. If our core theme broke under minimal pressure it would not be a good look for Drupal. That was the thinking.
Fast forward to today where that thinking has not been applied and we have breakages - for example Toolbar. It has an ID that could be used to isolate its styles, but it's not used, but this is one place where it should be. Now think about how many CSS related issues are posted against Bartik - virtually none. I hear people complain about Bartiks CSS but they all seem to forget how absolutely robust it turned out to be. Right?
My point is, yes remove them, given the idea that Bartik should become more of a base theme, however, do not make it so lame that it breaks the first time a contrib module does something weird and crazy, which of course, they do.
And fwiw, I do not consider or agree that id's are a bad practice, they can be badly used, but so can any technology. ID's have a purpose and its to give big specificity when required. Its what they were designed for.
I will say this again - yes I support removing them, but with caution against breakages, don't let it break easily, lots and lots of sites are going to use this theme.
Comment #99
mortendk CreditAttribution: mortendk commentedyeah gotta be honest i don't really care about bartik ;)
Comment #100
mgiffordOk, in that case.. Let's RTBC this issue.
Comment #103
alexpottSeems incompatible with the css rules in bartik's layout.css which use
#featured div.section
I really wish this followup had had a new issue. The original patch was committed in #34 we are now on comment #103. That's ridiculous.
Comment #104
Jeff Burnz CreditAttribution: Jeff Burnz commentedlol, yeah, its ridiculous, lets fix the selector, add the roles and be done with this, looking at the layout CSS makes me shudder, I can't bear it again. please, let this be over :/
Comment #105
sqndr CreditAttribution: sqndr commentedChanged the
layout.css
to be compatible with the new selector, as mentioned in #103.Comment #106
mgiffordThanks @sqndr - I prefer your JPG, but thought I'd include one that the UX folks could use.
Let's get this in and close this issue.
Comment #107
sqndr CreditAttribution: sqndr commentedWoops … Included the wrong file. Sorry about that. Thanks for creating a new screenshot @mgifford!
Comment #108
alexpottLet's end this. This issue is a unfrozen change as per https://www.drupal.org/core/beta-changes and it's benefits outweigh any disruption. Committed 7b1e2c1 and pushed to 8.0.x. Thanks!