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

Problem/Motivation

We aim to tweaks the styling of the navigation listings to bring them more inline with the new Seven visual language.

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:

The Nav List is used on “hub” pages to provide an on-page navigation menu with help text. This component is not greatly changed from D7, with only minor typographic finessing and a rounding of the “fancy bullet” used to mark the list items, in keeping with the friendliness of the brand.

Proposed resolution

In #1945552: Add Nav List component we laid the ground work for this change.

18.nav-list.png

Remaining tasks

  • Make a patch

User interface changes

This is a style only change,

API changes

No API Changes

Files: 
CommentFileSizeAuthor
#42 rtl-admin-config-before.png289.01 KBrteijeiro
#42 rtl-admin-config-after.jpeg257.52 KBrteijeiro
#42 rtl-admin-reports-before.png90.81 KBrteijeiro
#42 rtl-admin-reports-after.png93.78 KBrteijeiro
#41 ie8-nosvg.png13.84 KBechoz
#38 2016875-nav-list-38.patch5.87 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 58,785 pass(es).
[ View ]
#38 nav-list-svg-icon.png33.06 KBechoz
#32 no-desc-no-indent.png18.02 KBechoz
#31 2016875-nav-list-31.patch4.66 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 56,992 pass(es).
[ View ]
#28 2016875-nav-list-28.patch4.67 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 56,506 pass(es).
[ View ]
#28 admin-config-ltr.png32.19 KBechoz
#28 admin-config-rtl.png32.31 KBechoz
#27 2016875-nav-list-27.patch3.72 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 56,992 pass(es).
[ View ]
#24 s01-nopatcch-admin_config.png73.7 KBYesCT
#24 s01-patch-admin_config.png76.41 KBYesCT
#24 s01-patch-rtl-admin_config.png78.57 KBYesCT
#24 s01-nopatch-rtl-admin_config.png76.08 KBYesCT
#24 s02-nopatch-admin_reports.png34.78 KBYesCT
#24 s02-patch-admin_reports.png36.25 KBYesCT
#21 2016875-nav-list-21.patch3.62 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 56,417 pass(es).
[ View ]
#18 2016875-nav-list-13.patch3.62 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 56,748 pass(es).
[ View ]
#17 2016875-nav-list-17.patch9.74 KBrteijeiro
PASSED: [[SimpleTest]]: [MySQL] 56,392 pass(es).
[ View ]
#15 2016875-nav-list-14.patch9.74 KBrteijeiro
PASSED: [[SimpleTest]]: [MySQL] 56,390 pass(es).
[ View ]
#13 2016875-nav-list-13.patch3.62 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 56,376 pass(es).
[ View ]
#11 2016875-nav-list-11.patch3.97 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 58,376 pass(es).
[ View ]
#11 nav-list-config-rtl.png28.1 KBechoz
#8 2016875-nav-list-8.patch3.85 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 58,142 pass(es).
[ View ]
#8 nav-list-config.png29.02 KBechoz
#8 nav-list-reports.png18.27 KBechoz
#6 2016875-nav-list-6.patch3.69 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 58,192 pass(es).
[ View ]
#5 2016875-nav-list-5.patch3.98 KBLewisNyman
PASSED: [[SimpleTest]]: [MySQL] 57,956 pass(es).
[ View ]
#1 new-admin-list-style.patch3.68 KBLewisNyman
PASSED: [[SimpleTest]]: [MySQL] 57,723 pass(es).
[ View ]
18.nav-list.png18.92 KBLewisNyman

Comments

Status:Active» Needs review
StatusFileSize
new3.68 KB
PASSED: [[SimpleTest]]: [MySQL] 57,723 pass(es).
[ View ]

Here's the initial patch

Status:Needs review» Needs work
Issue tags:-Usability, -styleguide

The last submitted patch, new-admin-list-style.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Usability, +styleguide

#1: new-admin-list-style.patch queued for re-testing.

Status:Needs review» Needs work

Lookin' good.
* The .png is misnamed in the rtl file ("list-icon-rtl.png" rather than "nav-list-icon-rtl.png")
* git apply reports trailing whitespace on the pngs.

Status:Needs work» Needs review
StatusFileSize
new3.98 KB
PASSED: [[SimpleTest]]: [MySQL] 57,956 pass(es).
[ View ]

Thanks, I changed the class in the RTL file. Not sure about the whitespace in the png. I ran them through imageoptim....

StatusFileSize
new3.69 KB
PASSED: [[SimpleTest]]: [MySQL] 58,192 pass(es).
[ View ]

#5 patch changed the wrong unrelated img ruleset. Here is that correction. Patch has the same git warning, but applies and stages fine. Implementation looks fine.
-------

2016875-nav-list-6.patch:26: trailing whitespace.
?PNG
2016875-nav-list-6.patch:35: trailing whitespace.
?PNG
warning: 2 lines add whitespace errors.

Can we fix these whitespace errors?

The only thing I am concerned about is the fact that it vertically centers to the title. That looks a bit strange, but it looks totally fine without descriptions. Given that many content facing occasions there won't be descriptions. I am fine with leaving that, also with typographic changes it will look better.

StatusFileSize
new18.27 KB
new29.02 KB
new3.85 KB
PASSED: [[SimpleTest]]: [MySQL] 58,142 pass(es).
[ View ]

@Bojhan, not sure what you mean about "vertically centers to the title". With desciptions, I would think this is desired, same as previously.

The images were not transparent, they were on white. Since they are on a non white background on the config page, I've re-done the images from seventy-eight's PSD (plus optimized). I was hoping the whitespace warnings would go away, but they are the same. Note they are warnings not errors.

Only change in this patch is transparent images.

nav-list-config.pngnav-list-reports.png

The icon looks good aligned with the title, @Bojhan if you want to we could add a variant class if a description exists?

Middle alignment of icon and title, with or without a description is the way to go. I don't think a variant class is needed.

StatusFileSize
new28.1 KB
new3.97 KB
PASSED: [[SimpleTest]]: [MySQL] 58,376 pass(es).
[ View ]

Adjustment for previously untested RTL

nav-list-config-rtl.png

@echoz Nice work!

StatusFileSize
new3.62 KB
PASSED: [[SimpleTest]]: [MySQL] 56,376 pass(es).
[ View ]

StatusFileSize
new9.74 KB
PASSED: [[SimpleTest]]: [MySQL] 56,390 pass(es).
[ View ]

The patch :(

The last patch seems a little bit overreaching, should we make the changes here or as part of #2015789: Remove language_css_alter() (RTL stylesheets) in favor of HTML 'dir' attribute ?

StatusFileSize
new9.74 KB
PASSED: [[SimpleTest]]: [MySQL] 56,392 pass(es).
[ View ]

You are right @Lewis, I have included some crap from other patch, sorry :(

This is the right patch ;)

StatusFileSize
new3.62 KB
PASSED: [[SimpleTest]]: [MySQL] 56,748 pass(es).
[ View ]

Hold on. First of all, the patches in #15 or #17 (which are identical, btw) leave out the new icons. They are changing the rtl attribute selector in seven's style.css to have quotes. But #2030925: quote rtl attribute selector has not landed yet. Whichever patch gets committed first (2030925 or this one), the other will need to be re-rolled. The patch that will work right now is #13, which I am resubmitting here. If #2030925: quote rtl attribute selector gets committed before it, I will re-roll this with changing I think only one needed "rtl".

Status:Needs review» Reviewed & tested by the community

I agree with @echoz. Sorry for the noise :(

Patch applies well and seems to work well.

Issue tags:+RTBC July 1

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new3.62 KB
PASSED: [[SimpleTest]]: [MySQL] 56,417 pass(es).
[ View ]

Status:Needs review» Needs work
Issue tags:-Usability, -styleguide, -RTBC July 1

The last submitted patch, 2016875-nav-list-21.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Usability, +styleguide, +RTBC July 1

#21: 2016875-nav-list-21.patch queued for re-testing.

rtl is different than in #11.
is that because the fix for rtl will be in another issue?

admin/config

no patch

s01-nopatcch-admin_config.png

no patch - rtl

s01-nopatch-rtl-admin_config.png

patch

s01-patch-admin_config.png

patch - rtl

s01-patch-rtl-admin_config.png

admin/reports

no patch

s02-nopatch-admin_reports.png

patch

s02-patch-admin_reports.png

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

Yep, we could of easily lost some RTL styling in the past week.

Issue tags:+frontend, +D8MI

tagging for d8mi frontend people.

Status:Needs work» Needs review
StatusFileSize
new3.72 KB
PASSED: [[SimpleTest]]: [MySQL] 56,992 pass(es).
[ View ]

It was a specificity issue that my browser cache fooled me into thinking I had fixed.

StatusFileSize
new32.31 KB
new32.19 KB
new4.67 KB
PASSED: [[SimpleTest]]: [MySQL] 56,506 pass(es).
[ View ]

The column css was screwed up (previous to this issue) and issue only showed up on RTL. Fixed.

patched LTR

admin-config-ltr.png

patched RTL

admin-config-rtl.png

Status:Needs review» Needs work
Issue tags:-CSS, -Usability, -styleguide, -frontend, -D8MI, -css-novice, -RTBC July 1

The last submitted patch, 2016875-nav-list-28.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+CSS, +Usability, +styleguide, +frontend, +D8MI, +css-novice, +RTBC July 1

#28: 2016875-nav-list-28.patch queued for re-testing.

StatusFileSize
new4.66 KB
PASSED: [[SimpleTest]]: [MySQL] 56,992 pass(es).
[ View ]

Removed indentatation on "Hide description" display, as per #2037525: 'Hide description' layout bug

StatusFileSize
new18.02 KB

…and a screenshot

no-desc-no-indent.png

Status:Needs review» Needs work
Issue tags:-CSS, -Usability, -styleguide, -frontend, -D8MI, -css-novice, -RTBC July 1

The last submitted patch, 2016875-nav-list-31.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+CSS, +Usability, +styleguide, +frontend, +D8MI, +css-novice, +RTBC July 1

#31: 2016875-nav-list-31.patch queued for re-testing.

This still applies and should be easy to review (admin/config and admin/reports), can we move this along?

I swear I think someone at NYCCamp made all the before and after screenshots and double checked this...

This smells like RTBC

StatusFileSize
new33.06 KB
new5.87 KB
PASSED: [[SimpleTest]]: [MySQL] 58,785 pass(es).
[ View ]

I remember somewhere it was said we should leverage the icons from ry5n's Libricons #2032773: Use Libricons (icon font) in Seven, consider using it more broadly in core for the icons used here. In #1963886: Use HiDPI icons in the toolbar only the icons used in toolbar were added in misc/icons, which included chevron-disc up + down but not left + right, which are part of Libricons and the ones used here in the admin nav lists.

This new patch is the same as #31 patch, which still applys and was ready for rtbc, plus uses the svg (w/ Modernizer png fallback) from misc/icons.

It adds these icons, in the right color, at misc/icons:

chevron-disc-left.png
chevron-disc-left.svg
chevron-disc-right.png
chevron-disc-right.svg

It does not add the pngs (created from seventy-eight's PSDs) that had been used up to the #31 patch. If we decide not to use the svg / png from misc/icons, we can use the #31 patch.

Using svg icons from misc/icons

nav-list-svg-icon.png

tagging

Patch applies well (except for the errors regarding existing icon files) and looks really good.

Maybe it's a RTBC?

StatusFileSize
new13.84 KB

The git warnings (not errors) have always been there for the pngs.

I wanted to hear if nav-list is a candidate for switching to use svg icons, which I assume is true and what this last patch #38 does. Here's a screenshot showing that ie8 gets the Modernizer png fallback.

ie8-nosvg.png

RTL should be tested since this change (admin/config and admin/reports).

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new93.78 KB
new90.81 KB
new257.52 KB
new289.01 KB

It looks ok for me. Tested for RTL and it seems to fix a bug in admin/config page. Take a look at the screenshots:

admin/config BEFORE:

rtl-admin-config-before.png

admin/config AFTER:

rtl-admin-config-after.jpeg

admin/reports BEFORE:

rtl-admin-reports-before.png

admin/reports AFTER:

rtl-admin-reports-after.png

Well done echoz!!

Maybe RTBC?

Thanks for the RTL testing!
Correct, I discovered and fixed that column bug in #28 that existed previous to this issue but only showed up on RTL.

Just tested it again with LTR and RTL.
Still works - RTCB
No reroll needed.

Yes, this is ready to get in!

Status:Reviewed & tested by the community» Fixed

That looks much better!

Committed and pushed to 8.x. Thanks!

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

Issue summary:View changes

added related Use Libricons meta issue, removed 1849712