More Seven Theme issues: #1986434: [meta] New visual style for Seven

Problem/Motivation

As well as bringing the pagination more inline with new visual language for Seven, we hope to reduce the UI to serve the majority of cases in Drupal Core.

We developed Proposal: A Style Guide for Seven. This issue aims to introduce the proposed styling for pagination in core.

To quote the rationale provided:

To simplify the UI and focus on the 80% use case, this guide envisions removing the “first” and “last” links from the pagination component and also the text labels for “previous” and “next” links. This change is contingent on positive user testing.

Proposed resolution

In #1945548: Add pagination component we laid the ground work for this change.

17.pagination.png

Remaining tasks

  • Make a patch for CSS only changes
  • Investigate how we can utlise the views mini pager in core
  • Add icons in separate issue
  • Review accessibility

User interface changes

The 'first' and 'last' buttons will be removed. The labels for 'previous' and 'next' will be removed.

Test pages

  • admin/content (use devel generate to create more than 50 nodes)
Files: 
CommentFileSizeAuthor
#38 Pager in Seven theme.png192.81 KBmcjim
#33 pager.png2.46 KBrteijeiro
#29 new-pager-style-2016867-28.patch1.51 KBmcjim
PASSED: [[SimpleTest]]: [MySQL] 58,704 pass(es).
[ View ]
#29 interdiff-2016867-24-28.txt997 bytesmcjim
#29 2016867-28-screenshot.png12.05 KBmcjim
#24 interdiff.txt817 bytesLewisNyman
#24 new-pager-style-2016867-24.patch1.26 KBLewisNyman
PASSED: [[SimpleTest]]: [MySQL] 58,140 pass(es).
[ View ]
#19 Screen Shot 2013-07-29 at 21.26.28.png191.96 KBLewisNyman
#19 Screen Shot 2013-07-29 at 21.51.32.png184 KBLewisNyman
#16 new-pager-style-2016867-16.patch1.29 KBckrina
PASSED: [[SimpleTest]]: [MySQL] 57,984 pass(es).
[ View ]
#13 Captura de pantalla 2013-06-29 a les 11.46.49.png17.14 KBckrina
#12 new-pager-style-3.patch1.2 KBfrankbaele
PASSED: [[SimpleTest]]: [MySQL] 56,514 pass(es).
[ View ]
#11 pager_style_update.jpg32.86 KBmordonez
#10 new-pager-style-2.patch1.18 KBfrankbaele
PASSED: [[SimpleTest]]: [MySQL] 58,106 pass(es).
[ View ]
#1 new-pager-style.patch1.94 KBLewisNyman
PASSED: [[SimpleTest]]: [MySQL] 58,016 pass(es).
[ View ]
17.pagination.png5.95 KBLewisNyman

Comments

StatusFileSize
new1.94 KB
PASSED: [[SimpleTest]]: [MySQL] 58,016 pass(es).
[ View ]

Here's a CSS only patch

Status:Active» Needs review

Issue summary:View changes

Tweaks to proposed changes

Related issues:

#1488866: Redesign pagers for administrative lists.
#538788: Implement Seven pagers as designed

They were closed as duplicates, but they may contain some relevant insights.

This is a complete regression! If we go that route it should touch only Seven. Not the core pager.

Status:Needs review» Closed (duplicate)

This is the duplicate!

Status:Closed (duplicate)» Active

After a quick discussion with yoroy on IRC, we came to the conclusion that we should keep the scope of these two issues separate.

We can reduce the scope of this issue to implement the new Seven styling to the existing pagers with no changes to mark up.

This is a good news!

Theme and core needs to be kept separated.

Issue tags:+CSS, +Novice, +css-novice

Status:Active» Needs review
StatusFileSize
new1.18 KB
PASSED: [[SimpleTest]]: [MySQL] 58,106 pass(es).
[ View ]

css patch without touching system.base.css

Status:Needs review» Needs work
StatusFileSize
new32.86 KB

It seems necessary a bit more spacing on hover links

you can watch the review here https://saucelabs.com/tests/dfffc56bb14049f1a8d18a480b895f13

pager style

Status:Needs work» Needs review
StatusFileSize
new1.2 KB
PASSED: [[SimpleTest]]: [MySQL] 56,514 pass(es).
[ View ]

ok added some extra bottom padding to the focus & hover element

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new17.14 KB

Now working fine.

Status:Reviewed & tested by the community» Needs work

We are not far off! Could we try reducing the font-weight of the first/previous/next/last links? They seem to little bit too strong, the current pager item isn't as prominent as the original designs.

From a code perspective, there's only a few improvements I could find:

+++ b/core/themes/seven/style.cssundefined
@@ -481,6 +481,47 @@ ul.inline li {
+  transition: all 0.2s;

We need to add a -webkit prefixed property here I think

+++ b/core/themes/seven/style.cssundefined
@@ -481,6 +481,47 @@ ul.inline li {
+  font-size: 14px;
...
+  line-height: 22px;

We should use ems here

Assigned:Unassigned» ckrina

Starting to work on that.

Assigned:ckrina» Unassigned
Status:Needs work» Needs review
StatusFileSize
new1.29 KB
PASSED: [[SimpleTest]]: [MySQL] 57,984 pass(es).
[ View ]

Added both webkit and mozilla prefix.

Status:Needs review» Needs work
Issue tags:-CSS, -Usability, -Novice, -styleguide, -css-novice

The last submitted patch, new-pager-style-2016867-16.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+CSS, +Usability, +Novice, +styleguide, +css-novice

#16: new-pager-style-2016867-16.patch queued for re-testing.

Issue summary:View changes

Updated issue summary.

The patch applies well and the code looks good. Here's a before/after:

Screen Shot 2013-07-29 at 21.26.28.pngScreen Shot 2013-07-29 at 21.51.32.png

Status:Needs review» Reviewed & tested by the community

@LewisNyman: Where you thinking of setting this to RTBC?

#16: new-pager-style-2016867-16.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs review

Sorry, I need a stronger RTBC than that, from someone who's reviewed the CSS line-by-line and can vouch for it being correct.

Should this need a re-roll:

+++ b/core/themes/seven/style.css
@@ -481,6 +481,50 @@ ul.inline li {
+/**
+* Pagination.
+* The item-list CSS uses quite strong selectors, we have to match them here to override.
+*/

The starred lines under /** should be indented one space so they line up. And the comment should wrap at 80 characters. I can always fix it on commit, too.

Status:Needs review» Needs work

We all can! :-)

StatusFileSize
new1.26 KB
PASSED: [[SimpleTest]]: [MySQL] 58,140 pass(es).
[ View ]
new817 bytes

I've made a few minor modifications to the code and made the changes webchick mentioned.

Status:Needs work» Needs review

Status:Needs review» Reviewed & tested by the community

Works as described in #2016867-19: Pager Style Update. It's a RTBC for me and looks shiny! :)

Status:Reviewed & tested by the community» Needs work

It looks like the existing CSS definition in seven's style.css can be removed.

.item-list .pager li {
  padding: 0.5em;
}

Assigned:Unassigned» mcjim

Status:Needs work» Needs review
StatusFileSize
new12.05 KB
new997 bytes
new1.51 KB
PASSED: [[SimpleTest]]: [MySQL] 58,704 pass(es).
[ View ]

Rerolled patch to:

  • remove existing CSS definition (#27)
  • change the transition on hover from all to border-bottom-color after Lewis pointed that out
  • move the padding for the a element hover state only to the a element in all states to avoid any oddities with the transition (and as good practice, to stop the possibility of anything changing position as a result of changing states)
  • amend the padding bottom to 3px from 4px to line things up properly on the hover state

That looks more than it actually is :-)

Assigned:mcjim» Unassigned

Status:Needs review» Needs work
Issue tags:-CSS, -Usability, -Novice, -styleguide, -css-novice

The last submitted patch, new-pager-style-2016867-28.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+CSS, +Usability, +Novice, +styleguide, +css-novice

#29: new-pager-style-2016867-28.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new2.46 KB

Patch applies well and looks great! Maybe RTBC when testbot agree with it :P

pager.png

Status:Reviewed & tested by the community» Needs work

Test failed.

Status:Needs work» Needs review

Status:Needs review» Reviewed & tested by the community

Test is green.

Status:Reviewed & tested by the community» Needs work

Looked this over with webchick and we're not seeing the underline on the active item in Firefox nor Chrome. Tested locally and on simplytest.me so there seems to be something missing in the patch.

StatusFileSize
new192.81 KB

This is a screenshot of Chrome from a simplytest.me install.
Pager in Seven theme

(If you look closely at the bottom, you'll also see the pager from Barktik under the overlay. Unlike Seven, this doesn't have the underline for the current page.)

Status:Needs work» Reviewed & tested by the community

Doh. And you specifically asked if we were looking at Seven. We weren't. Sorry! Back to rtbc.

Status:Reviewed & tested by the community» Fixed

Committed a534266 and pushed to 8.x. Thanks!

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

Issue summary:View changes

Added test page