Problem/Motivation

With Seven now using Normalize.css for its reset, there are some minor visual regressions to fix (none of which affect functionality). Regressions that potentially affect the usability are marked in bold.

Misc. UI

  • Dropbutton arrow icon is mis-aligned (img)
  • Simpletest pages add an underline to the acive breadcrumb link (img)
  • On admin/reports/updates, bullets should not appear on the 'download/release' notes list items. (img)

Forms

  • text inputs and selects inherit body font and size, so are now larger and in Lucida instead of Helvetica (img)
  • Reduced vertical space between top-aligned labels and their form-items
  • textareas inherit body font and size and have smaller line-height (img)

Typography

  • Numbered and bulleted lists have too much side margin/padding. Amount of extra margin not consistent in all contexts. (img)
  • Numbered and bulleted lists have a small amount of extra vertical margin between list items (or larger line-height, TBD).

Tables

  • Table rows in admin/people/permissions have extra top and bottom padding. (img)

Blocks UI

  • On admin/structure/block, block table contains extra borders on table rows. Also a small amount of unnecessary extra vertical space within table rows. (img)

Field UI

  • Form items withn Field UI tables have too much vertical margin.
  • Some tables in Field UI add an unnecessary top border to the last table row.

Views UI

  • Some (but not all) radio groups in Views UI have unnecessary extra vertical space between radio items.

Proposed resolution

Many or most of these regressions are minor, and may become moot as features and UIs are updated. The only one of these that affects usability or functionality at all is the Blocks UI issue, so this should be fixed now, and other regressions fixed later, after feature-freeze.

Remaining tasks

  • Post patch fixing the Blocks UI regression.
  • After feature freeze, fix any regressions that remain, or fix as part of completing other improvements to Core.
CommentFileSizeAuthor
#7 After-Chrome-Blocks page.JPG69.47 KBShyamala
#7 After-Chrome-Help.JPG84.86 KBShyamala
#7 After-Chrome-dropbuttons.JPG4.49 KBShyamala
#7 After-Chrome-No -Wierd-Bullets.JPG77.84 KBShyamala
#7 After-IE-Blocks page.JPG99.59 KBShyamala
#7 After-IE-dropbuttons.JPG4.16 KBShyamala
#6 normalize--admin-structure-block.png193.35 KBry5n
#6 reset--admin-stucture--block.png194.72 KBry5n
#6 reset--admin-help.png203.19 KBry5n
#4 core--fix-normalize-fallout--1837514--3.patch3.15 KBry5n
seven-blocks-ui.png123.47 KBry5n
permissions-table.png33.8 KBry5n
admin-help-lists.png132.86 KBry5n
seven-forms-textarea.png28.59 KBry5n
seven-forms.png39.64 KBry5n
admin-reports-updates.png74.41 KBry5n
seven-dropbutton.png5.03 KBry5n
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented
Issue tags: +Usability

I would call the views UI issue quite a regression too, you often cant even see which belong together anymore.

ry5n’s picture

ry5n CreditAttribution: ry5n commented

@Bojhan I did quite extensive testing to come up with the list above, including testing Views UI. I used Kaleidoscope.app to do visual diffs between screenshots taken before/after the Normalize patch. I did not see any changes to Views UI from the Normalize patch other than the form element changes listed above. Is is possible that the regressions to Views are related to another patch? @tim.plunkett suggests they were; see http://drupal.org/node/723392#comment-6697098.

That said, we should fix all of these problems no matter the origin. I definitely see where Views UI and other places look wrong. EDIT: I would also be happy to see this issue encompass any visual regressions from other patches; it probably makes sense to tackle them all in one place.

For now, I want to spend our time where it counts, and I think the majority of these fixes should come later.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Please note that most of the admin table/form item issues are resolved via #1168246: Freedom For Fieldsets! Long Live The DETAILS.

That is, because Seven overrides system.theme.css' .form-item styling to use vertical padding instead of margin to achieve spacing between form items, but the new details don't involve any quirks in any browser, so their excessive styling in Seven was able to be simplified a lot; going back to using the vertical margin spacing from system.theme.css.

That resolves the layout/styling of all form items, but also all administrative tables (e.g., Block UI), as the additional spacing is caused by the vertical padding (which was overridden through Seven's former reset.css previously).

I'd therefore strongly recommend to wait with fixes here until aforementioned patch has been committed.

Additionally, the table row border was brought up in #1813792-16: Remove ugly default CSS styles for table already, but I inspected the situation and think that the (originally unintended) change is actually not a regression, but a helpful visual guidance. Open to further discuss this here, of course.

ry5n’s picture

ry5n CreditAttribution: ry5n commented
Title: Fix minor visual regressions from taming Seven’s reset » Fix visual regressions from taming Seven’s reset
FileSize
core--fix-normalize-fallout--1837514--3.patch3.15 KB

This patch makes the following elements in Seven consistent with the state before the normalize patch:

- Blocks UI
- ol and ul
- dropbuttons (which should be more resilient in future)
- link list in update module (admin/reports/updates)
- line-height on textareas

ry5n’s picture

ry5n CreditAttribution: ry5n commented
Status: Active » Needs review

Needs testing.

ry5n’s picture

ry5n CreditAttribution: ry5n commented
FileSize
reset--admin-help.png203.19 KB
reset--admin-stucture--block.png194.72 KB
normalize--admin-structure-block.png193.35 KB

(Some help for reviewers.) You can test this patch by viewing the following pages:

  • For the Blocks UI: admin/structure/block. It should look like this screenshot from before the patch.
  • For lists: admin/help. Should look like this.
  • For dropbuttons, check the Views UI and eg. admin/structure/types
  • For the update UI, ensure there are no weird bullets in the middle of the tables on admin/reports/updates
  • For textareas, e.g. on admin/node/add/article, ensure that line-height is generous (computed value should be about 1.5em)
Shyamala’s picture

Shyamala CreditAttribution: Shyamala commented
FileSize
After-IE-dropbuttons.JPG4.16 KB
After-IE-Blocks page.JPG99.59 KB
After-Chrome-No -Wierd-Bullets.JPG77.84 KB
After-Chrome-dropbuttons.JPG4.49 KB
After-Chrome-Help.JPG84.86 KB
After-Chrome-Blocks page.JPG69.47 KB
  • For the Blocks UI: admin/structure/block - Looks good, attached screenshot in Chrome and IE8
  • For lists: admin/help. Should look like this. - Looks good, attached screenshot in Chrome and IE8
  • For dropbuttons, check the Views UI and eg. admin/structure/types - Looks good, attached screenshot in Chrome
    In IE 8 the second dropdown is partly hidden.
  • For the update UI, ensure there are no weird bullets in the middle of the tables on admin/reports/updates - Looks good, attached screenshot in Chrome and IE8
  • For textareas, e.g. on admin/node/add/article, ensure that line-height is generous (computed value should be about 1.5em) - Looks good in Chrome and IE8
ry5n’s picture

ry5n CreditAttribution: ry5n commented

@Shyamala Thanks for the review! I will take a closer look at the dropbutton and fix whatever is going on there.

rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro commented
Status: Needs review » Fixed

It seems that all mentioned issues have been already solved so this patch is no longer needed. Should I close this issue as fixed?

rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro commented
Status: Fixed » Reviewed & tested by the community

Sorry, maybe it's better mark it as RTBC?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Patch no longer applies.

rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro commented
Assigned: Unassigned » rteijeiro

Re-rolling...

rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro commented
Status: Needs work » Reviewed & tested by the community

@alexpott: Regarding #9 comment, this patch is no longer needed because all the issues are already fixed by the new Seven style issues. So no need to re-roll the patch.

Ping me in IRC if you want to discuss it. I am in the sprint room with LewisNyman ;)

rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro commented
Status: Reviewed & tested by the community » Closed (duplicate)

Closing as duplicated after discussing with alexpott :)

mcrittenden’s picture

mcrittenden CreditAttribution: mcrittenden commented
Issue tags: -Needs reroll

Tags