Problem/Motivation
Seven has a handful of CSS that assumes desktop-sized layouts. We need to refactor those to use mobile-first approach. While looking through all of Seven's styles we found that 2 groups of CSS rulesets that use desktop-sized widths are:
- #branding div.block {}
- #secondary-links ul.links li {}
Interestingly, the CSS rulesets in #1 and #2 don't seem to apply to actual markup that is present in Seven.
In #1, you can't add a block to the #branding div of page.tpl because its not a region. In #2, the page.tpl.php does not print out the secondary menu links that other themes do (like Bartik), so that ruleset will never match anything. Even if you disable the Toolbar module, Seven doesn't show any secondary menu links.
Proposed resolution
We should re-focus this issue on just removing those 2 groups of CSS.
Remaining tasks
Write the patch!
Original report
When the width of the browsing window falls below 745px the Seven theme really struggles to display content and navigation in a decent manner.
We need to go through all the pages and restyle them to make the most of limited real estate.
Comment | File | Size | Author |
---|---|---|---|
#18 | 1137782-18-seven-remove-css.patch | 2.2 KB | dcam |
#8 | drupal-remove_css-1137782-8.patch | 2.21 KB | disasm |
Comments
Comment #1
LewisNyman CreditAttribution: LewisNyman commentedComment #2
Devin Carlson CreditAttribution: Devin Carlson commentedDo you have a reason for supporting resolutions below 745px?
Every recent statistic that I've seen (including my own data) shows that resolutions below 1024x768 are not used except for on mobile devices (which should have their own custom theme anyways).
w3schools lists 0% of site visitors having a resolution below 1024x768 as of January 2011 (http://www.w3schools.com/browsers/browsers_display.asp).
Even the government clients that I work with rarely see a visitor with 800x600 or lower (less than 1% of all traffic).
Comment #3
LewisNyman CreditAttribution: LewisNyman commentedIt seems like a big assumption that mobile devices need their own separate theme. It gets muddy when you need to start defining 'mobile' and how other devices like tablets start fitting in to this solution. Do we need to switch the theme when a device switches from portrait to landscape? It's a lot of overhead.
Do we need UA sniffing and theme switching functionality in core? I don't think so. Doing the detection on the front end is faster and cleaner.
Comment #4
Devin Carlson CreditAttribution: Devin Carlson commentedVery true, the lines between what is and isn't mobile are definitely getting blurrier with each new generation of hardware. I agree that it would be a bad idea to have UA detection in core or to have to support however many user agents that are currently in use around the world.
My point is that all desktop/laptops now have a screen resolution greater than 1024x768 and most recent smartphones/tablets already have a resolution greater than 745px (iPhone 4 960x640, Samsung Galaxy S 800x480, Samsung Galaxy Tab 1280x800), at least vertically (and pretty much all new smartphones can be used in landscape mode).
I would still argue that feature phones and embedded devices of similar power should have their own theme/stylesheet/etc in order to best make use of their limited real estate but it isn't core's responsibility to support these devices.
I think that the Web Services and Context Core Initiative (http://groups.drupal.org/wscci), once finished, will be a great tool for anyone who has to support special device use cases.
Comment #5
JohnAlbinThe content editing and site administering part of Drupal are the first things people see on a new Drupal install. First impressions matter, so I think having Seven be responsive to any screen size is essential.
Comment #6
jefflinwood CreditAttribution: jefflinwood commentedSpecifically, the CSS in Seven's style.css that should change along with screen width is Lines 734-752:
That said, it's my recommendation that we look at a semantic way of describing this output.
Comment #7
JohnAlbinRe: comment #6: We added media queries to the admin left/right columns over in #1150134: Admin Panels — Remove Columns for screen widths under 600px
I went through all of Seven's stylesheets and discovered these CSS issues that have a fixed width that would be problematic in smaller screens:
There are existing issues for #1 and #2. Interestingly, the CSS rulesets in #3 and #4 don't seem to apply to actual markup that is present in Seven.
In #3, you can't add a block to the #branding div of page.tpl because its not a region. In #4, the page.tpl.php does not print out the secondary menu links that other themes do (like Bartik), so that ruleset will never match anything. Even if you disable the Toolbar module, Seven doesn't show any secondary menu links.
So we should re-focus this issue on just removing those 2 groups of CSS.
Comment #8
disasm CreditAttribution: disasm commentedThis patch covers 3 and 4. Since the comment in #7 said neither apply to markup, I removed everything regarding those css rules and not just the fixed layout stuff.
Comment #9
Bojhan CreditAttribution: Bojhan commentedI dont think you meant to assign?
Comment #10
LewisNyman CreditAttribution: LewisNyman commentedI reviewed the patch and I can't find any negative effects from the removal of these styles. I'm also stumped as to why they are in there in the first place. I had a click around a broad selection of admin pages and everything looks fine.
Can we get someone who is more familiar with the history of Seven? If not I'm happy with RTBC.
Comment #11
mcjim CreditAttribution: mcjim commentedNot familiar with history of Seven, but on looking at it,
#branding div.block
doesn't appear to be needed.Only things printed out in the #branding div are breadcrumb, title and primary local tasks. There is no region and no blocks should ever be output here.
Agree that
#secondary-links ul.links
is also not needed.Patch applies nicely. No side affects noticed on a random of admin pages, including node/add form, admin, admin/config, admin/structure/taxonomy.
Comment #12
JohnAlbinI am familiar with the history of Seven. It was developed on the moving target of Drupal 7's default markup. And it had features that it had pre-commit that were later removed. I'm sure the unnecessary CSS is an artifact of the redesign of Seven's features.
So, RTBC+1. :-)
Comment #13
JohnAlbin#8: drupal-remove_css-1137782-8.patch queued for re-testing.
Comment #14
catchCommitted/pushed to 8.x, thanks!
Comment #16
Albert Volkman CreditAttribution: Albert Volkman commentedThis exists in D7, should it be backported?
Comment #17
JohnAlbinComment #18
dcam CreditAttribution: dcam commentedBackported #8 to D7.
Comment #18.0
dcam CreditAttribution: dcam commentedAdd proper issue summary
Comment #19
LewisNyman CreditAttribution: LewisNyman commented18: 1137782-18-seven-remove-css.patch queued for re-testing.
Comment #20
LewisNyman CreditAttribution: LewisNyman commentedComment #21
LewisNyman CreditAttribution: LewisNyman commentedI don't think this is the right way to go about it now. We should stick to working on CSS on a per-file basis instead of global changes.