More Seven Theme issues: #1986434: 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.
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)
Related Issues
Comment | File | Size | Author |
---|---|---|---|
#38 | Pager in Seven theme.png | 192.81 KB | mcjim |
#33 | pager.png | 2.46 KB | rteijeiro |
#29 | new-pager-style-2016867-28.patch | 1.51 KB | mcjim |
#29 | interdiff-2016867-24-28.txt | 997 bytes | mcjim |
#29 | 2016867-28-screenshot.png | 12.05 KB | mcjim |
Comments
Comment #1
LewisNyman CreditAttribution: LewisNyman commentedHere's a CSS only patch
Comment #2
LewisNyman CreditAttribution: LewisNyman commentedComment #2.0
LewisNyman CreditAttribution: LewisNyman commentedTweaks to proposed changes
Comment #3
LewisNyman CreditAttribution: LewisNyman commentedRelated 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.
Comment #4
xmacinfoThis is a complete regression! If we go that route it should touch only Seven. Not the core pager.
Comment #5
xmacinfoThis is the duplicate!
Comment #6
yoroy CreditAttribution: yoroy commentedContinue in #538788: Implement Seven pagers as designed
Comment #7
LewisNyman CreditAttribution: LewisNyman commentedAfter 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.
Comment #8
xmacinfoThis is a good news!
Theme and core needs to be kept separated.
Comment #9
LewisNyman CreditAttribution: LewisNyman commentedComment #10
frankbaele CreditAttribution: frankbaele commentedcss patch without touching system.base.css
Comment #11
mordonez CreditAttribution: mordonez commentedIt seems necessary a bit more spacing on hover links
you can watch the review here https://saucelabs.com/tests/dfffc56bb14049f1a8d18a480b895f13
Comment #12
frankbaele CreditAttribution: frankbaele commentedok added some extra bottom padding to the focus & hover element
Comment #13
ckrinaNow working fine.
Comment #14
LewisNyman CreditAttribution: LewisNyman commentedWe 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:
We need to add a -webkit prefixed property here I think
We should use ems here
Comment #15
ckrinaStarting to work on that.
Comment #16
ckrinaAdded both webkit and mozilla prefix.
Comment #18
rteijeiro CreditAttribution: rteijeiro commented#16: new-pager-style-2016867-16.patch queued for re-testing.
Comment #18.0
rteijeiro CreditAttribution: rteijeiro commentedUpdated issue summary.
Comment #19
LewisNyman CreditAttribution: LewisNyman commentedThe patch applies well and the code looks good. Here's a before/after:
Comment #20
xmacinfo@LewisNyman: Where you thinking of setting this to RTBC?
Comment #21
xjm#16: new-pager-style-2016867-16.patch queued for re-testing.
Comment #22
webchickSorry, 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:
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.
Comment #23
xmacinfoWe all can! :-)
Comment #24
LewisNyman CreditAttribution: LewisNyman commentedI've made a few minor modifications to the code and made the changes webchick mentioned.
Comment #25
LewisNyman CreditAttribution: LewisNyman commentedComment #26
rteijeiro CreditAttribution: rteijeiro commentedWorks as described in #2016867-19: Pager Style Update. It's a RTBC for me and looks shiny! :)
Comment #27
alexpottIt looks like the existing CSS definition in seven's style.css can be removed.
Comment #28
mcjim CreditAttribution: mcjim commentedComment #29
mcjim CreditAttribution: mcjim commentedRerolled patch to:
all
toborder-bottom-color
after Lewis pointed that outThat looks more than it actually is :-)
Comment #30
mcjim CreditAttribution: mcjim commentedComment #32
LewisNyman CreditAttribution: LewisNyman commented#29: new-pager-style-2016867-28.patch queued for re-testing.
Comment #33
rteijeiro CreditAttribution: rteijeiro commentedPatch applies well and looks great! Maybe RTBC when testbot agree with it :P
Comment #34
xmacinfoTest failed.
Comment #35
LewisNyman CreditAttribution: LewisNyman commentedComment #36
rteijeiro CreditAttribution: rteijeiro commentedTest is green.
Comment #37
yoroy CreditAttribution: yoroy commentedLooked 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.
Comment #38
mcjim CreditAttribution: mcjim commentedThis is a screenshot of Chrome from a simplytest.me install.
(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.)
Comment #39
yoroy CreditAttribution: yoroy commentedDoh. And you specifically asked if we were looking at Seven. We weren't. Sorry! Back to rtbc.
Comment #40
alexpottCommitted a534266 and pushed to 8.x. Thanks!
Comment #41.0
(not verified) CreditAttribution: commentedAdded test page