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:

  1. #branding div.block {}
  2. #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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

Issue tags: +d8mux
Devin Carlson’s picture

Do 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).

LewisNyman’s picture

It 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.

Devin Carlson’s picture

Very 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.

JohnAlbin’s picture

Title: Improve support for resolutions between 320px and 745px » Improve Seven theme's support for smaller screen sizes
Issue tags: -#d8ux +mobile, +d8ux

The 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.

jefflinwood’s picture

Specifically, the CSS in Seven's style.css that should change along with screen width is Lines 734-752:

/**
 * System.
 */
div.admin .right,
div.admin .left {
  width: 49%;
  margin: 0;
}
div.admin-panel,
div.admin-panel .body {
  padding: 0;
  clear: left;
}
div.admin-panel {
  margin: 0 0 20px;
  padding: 9px;
  background: #f8f8f8;
  border: 1px solid #ccc;
}

That said, it's my recommendation that we look at a semantic way of describing this output.

JohnAlbin’s picture

Title: Improve Seven theme's support for smaller screen sizes » Remove unused CSS in Seven theme that uses desktop-sized layouts
Issue tags: +mobile-novice

Re: 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:

  1. #1661814: Make "in maintenance" Seven use a responsive design
  2. #1277352: Responsive vertical tabs
  3. #branding div.block {}
  4. #secondary-links ul.links li {}

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.

disasm’s picture

Assigned: Unassigned » disasm
Status: Active » Needs review
FileSize
2.21 KB

This 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.

Bojhan’s picture

Assigned: disasm » Unassigned

I dont think you meant to assign?

LewisNyman’s picture

I 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.

mcjim’s picture

Status: Needs review » Reviewed & tested by the community

Not 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.

JohnAlbin’s picture

I 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. :-)

JohnAlbin’s picture

#8: drupal-remove_css-1137782-8.patch queued for re-testing.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Albert Volkman’s picture

Status: Closed (fixed) » Patch (to be ported)

This exists in D7, should it be backported?

JohnAlbin’s picture

Version: 8.x-dev » 7.x-dev
dcam’s picture

Status: Patch (to be ported) » Needs review
FileSize
2.2 KB

Backported #8 to D7.

dcam’s picture

Issue summary: View changes

Add proper issue summary

LewisNyman’s picture

LewisNyman’s picture

Issue summary: View changes
Issue tags: +CSS, +frontend
LewisNyman’s picture

Status: Needs review » Closed (won't fix)

I 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.