Originally submitted on Github

Problem/Motivation

Recent interviews and research exposed pain points around Drupal's admin experience of looking and feeling dated.

Proposed resolution

Implement new Accordion/Sidebar styles to create a favorable first impression of Drupal for evaluators and a better user experience for site authors. No functional differences.

Specification

Quick overview

This image is just a quick overview for Accordion/sidebar specs. Please use the Figma link to full specification as the main resource for specks.
Accordion styles

Full specification

FIGMA: https://www.figma.com/file/OqWgzAluHtsOd5uwm1lubFeH/Drupal-Design-system...
This link is anchored to the board with the full specification. As an anonymous user you can see the design, but to actually be able to pick colours and sizes please login to Figma.

Remaining tasks

  • Update patch with new styles implemented
  • Accessibility review
  • RTL review (Right to left)

User interface changes

All Accordion/Sidebar styles will be changed, no functional differences.

Test Pages

  • /node/add/article
  • /node/add/page
  • /admin/modules
  • /user/1/edit?destination=/admin/people
  • /admin/people/create
  • /admin/modules

Node add/edit page design: here.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

antonellasevero created an issue. See original summary.

saschaeggi’s picture

Version: » 8.x-1.x-dev
Status: Active » Postponed

Missing final specs

huzooka’s picture

ckrina’s picture

Component: Code » Needs design
Assigned: Unassigned » ckrina
Issue summary: View changes
ckrina’s picture

Title: Details/Accordion/Sidebar style update » Accordion/Sidebar style update
Issue summary: View changes
ckrina’s picture

Component: Needs design » Code
Assigned: ckrina » Unassigned
Issue summary: View changes
FileSize
174.98 KB

Adding final design specs so it's ready to develop.

ckrina’s picture

Status: Postponed » Active

I forgot to change status.

huzooka’s picture

Status: Active » Postponed
Parent issue: » #3038784: Details style update
Related issues: +#3038784: Details style update

Even that we have design now, I'll postpone this until #3038784: Details style update is merged since these elements are details.
We should wait for #3038784: Details style update to prevent double-coding (mostly) the same thing twice.

ckrina’s picture

huzooka’s picture

Assigned: Unassigned » huzooka
Status: Postponed » Needs work

I'm working on this.

huzooka’s picture

Assigned: huzooka » Unassigned
Status: Needs work » Needs review
FileSize
23.01 KB

Node add/edit sidebar is ready for the first review.

Screenshots in progress.

huzooka’s picture

Assigned: Unassigned » huzooka
Status: Needs review » Needs work

I found a smaller bug...

huzooka’s picture

Assigned: huzooka » Unassigned
Status: Needs work » Needs review
FileSize
23.01 KB
394 bytes
lauriii’s picture

Status: Needs review » Needs work

Tested manually with Firefox, Chrome, Safari, IE 11 (including high contrast) and Edge (including high contrast) and everything seems to be working consistently ✨

  1. +++ b/claro.theme
    @@ -308,6 +344,23 @@ function claro_preprocess_maintenance_page(&$variables) {
    +  if (!empty($element['#accordion_item'])) {
    ...
    +    $variables['accordion_item'] = TRUE;
    ...
    +  if (!empty($element['#accordion'])) {
    ...
    +    $variables['accordion'] = TRUE;
    

    We should open an issue in Drupal core for creating a new accordion render element. Let's also add @todo mentioning the Drupal core issue.

  2. +++ b/css/src/components/details.css
    @@ -217,21 +328,53 @@
    + * wrapper for settings the background color.
    

    s/settings/setting

  3. +++ b/css/src/components/details.css
    @@ -217,21 +328,53 @@
    + * Using margins instead of padding is much better since the elements inside the
    + * details won't cause too big top or bottom spacing.
    

    I like the fact that this comment is explaining why since that's usually always the most valuable information to include in a comment 👍 I'm just wondering there is a better way to phrase this. We should at least mention that colliding vertical margins collapse and we want to take benefit of that behavior.

  4. +++ b/css/src/components/vertical-tabs.css
    @@ -6,11 +6,15 @@
    +  margin: 10px 0; /* Stable theme has a stronger RTL selector */
    

    Should we override that CSS? Are we using anything from there?

  5. +++ b/templates/details.html.twig
    @@ -19,33 +23,65 @@
    +    <div{{ create_attribute({class: inner_wrapper_classes }) }}>
    

    Nit: according to Twig coding standards, there shouldn't be whitespace after inner_wrapper_classes.

huzooka’s picture

Assigned: Unassigned » huzooka
huzooka’s picture

huzooka’s picture

Assigned: huzooka » Unassigned
FileSize
23.25 KB
2.31 KB

Addressing #14.2, #14.3, #14.4 and #14.5.

To fulfill #14.1, we need a clear 'motivation' section for the core feature request — to prevent misunderstanding.
(Right now my interpretation is that the Accordion may be used as a standalone UI element as well.)

huzooka’s picture

Status: Needs work » Needs review
huzooka’s picture

Status: Needs review » Needs work

Back to needs work (for #15.1).

huzooka’s picture

Screenshots generated with patch #17 attached.

lauriii’s picture

@huzooka that was the motivation for opening the issue. We should open issue for standardizing the way accordion markup gets rendered so that it could be used elsewhere. It's not a blocker and I don't think we even have to build support for that in Claro, but it would be nice to have at some point.

huzooka’s picture

huzooka’s picture

Assigned: huzooka » Unassigned
Status: Needs work » Needs review
FileSize
23.4 KB
2.31 KB

This patch addresses #15.1 as well, so we are ready for a next review.

ckrina’s picture

Sorry for the late comment, but yeah on a UI perspective this is a component that we might use in other places. We don't have, right now, this "new use case" other than the sidebar, but I'd say that on a frontend/design implementation perspective it'd be already a separated component. So +1 on the direction taken :)

lauriii’s picture

Status: Needs review » Needs work
FileSize
48.82 KB
82.48 KB

Patch currently:

Styleguide:

Should we fix some of the details in the entity meta element on this issue as well? The issues I can see:

  • Last saved is italic with the current patch but in design its normal text
  • There's no margin between the author and the revision log message. I'm wondering if it should be after the author or before revision message because on edit pages there's a "Create new revision" in between these elements.
  • It seems like the entity meta is supposed to have a white background, instead of the light gray.
huzooka’s picture

Lauri, where can I find the styleguide you mentioned in #25?

huzooka’s picture

Assigned: Unassigned » huzooka
Issue summary: View changes

I found a node edit page design... I added it to the issue summary.

huzooka’s picture

lauriii’s picture

Issue summary: View changes
Status: Needs review » Needs work
FileSize
50.88 KB

Looks already much better! I think we should still adjust the margin on top of the revision message to match the designs:

huzooka’s picture

The extra space should be added between the group of the revision inputs (checkbox + message) and the publishing group (changed date + author).

lauriii’s picture

Assigned: Unassigned » lauriii
lauriii’s picture

Assigned: lauriii » Unassigned
Issue summary: View changes
Status: Needs work » Needs review
FileSize
24.99 KB
837 bytes
49.43 KB

This addresses #29.

lauriii’s picture

lauriii’s picture

Status: Needs review » Reviewed & tested by the community

I think this is ready to be committed. My changes to the patch were relatively small, but I'm still waiting for confirmation from @huzooka that my changes are fine.

huzooka’s picture

#33 is fine, I agree with the RTBC.

  • lauriii committed c22cb93 on 8.x-1.x
    Issue #3023309 by huzooka, lauriii, ckrina, antonellasevero, saschaeggi...
lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Committed! Thank you everyone! ✌️

Status: Fixed » Closed (fixed)

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