Add workspace UI in top dialog

I'd like to treat #2940677: Support prefers-reduced-motion in off-canvas dialog as a should-have for the Workspace UI.

WCAG 2.1 brings in a new "animations from interactions" success criteria, to support users who have difficulty with vestibular disorders, etc.. The off-canvas dialog is probably a high-risk animation for users affected by these - I'm going by the fact that it involves a large portion of the viewport, especially the top edge demos we've seen for Workspace UI. Easing effects would also be a contributing factor.

Now it's coming in at WCAG level AAA, which is beyond our target of AA which our accessibility QA gate requires. So that means it's NOT a must-have blocker. But it is high impact for affected users, and looks like it will be easy to implement. So we should ;-)

Is this the right issue to track the must/should/could issues for the Workspace UI stability roadmap?

Over at #2928103: [policy, no patch] Use "prefers-reduced-motion" media query to disable animations we're forming a more detailed plan about how to standardize our approach to supporting reduced motion. I think we ought to have a principle of making sure our UI designs don't rely on animations, just as they shouldn't require full-colour vision, say. From what I've seen so far, the Workspace UI doesn't have anything which relies on animation to to convey important animation.

Note that Settings Tray is already marked stable, and has an animation when opening. I was thinking about prefers-reduced-motion as a nice-to-have for stabilizing that module. But some of the next round of experimental modules for the authoring experience are proposing to make more use of the off-canvas, using bigger portions of the viewport too. So that's why I'd like to start treating support for prefers-reduced-motion as a should-have issue. (I'll make a similar argument about the proposal to use bigger off-canvas dialogs for creating blocks in the layout builder initiative.)

There are some WCAG issues relating to colour in the workspace UI designs. I made a long comment about this at #2910673-10: Workspace UI usability review.

• SC 1.4.1 Use of color - the "view all workspaces" screen seems to convey workspace status by colour alone.
• SC 1.4.3 Contrast (minimum) - we'll need to ensure that all the workspace status colours provide sufficient contrast wherever they are used a text. It could be tricky to find colours which have sufficient contrast when paired with white AND dark grey.
• WCAG 2.1 introduces SC 1.4.11 Non-text contrast. Where the status icons are presented on their own (without the status text label) they'll need at least 3:1 contrast. That's less than the text contrast requirement, so if the colours satisfy the text contrast, we've probably got this one covered too.

Question: can site-builders define their own custom workspace statuses? If users define the colours, we can't be sure the UI will satisfy either of these WCAG criteria. All we can do in that case is provide good default colours.

Adding this feature is a really good improvement because it can add a useful and well know pattern along the web and in apps. My main concern though is how it looks/behaves in small/thinner devices. I'm trying to reproduce it locally but since I don't have enough experience in Workspaces I don't know how to set everything to see it working live. Maybe an screnshot/gif for small and/or medium devices would help.

Also, related to #2916781: Allow off-canvas dialog to be rendered at the top of the page as this is the implementation, do you think that a pattern or direction could be set down here (in terms of using columns or responsive behaviour) for any other modules or future feature to follow later on?

Here is the latest theming progress. It includes improvised styles for the mobile viewport as seen above, but no breakpoints in between at this time.

interdiff-2949991-14.txt

Rerolled the patch from #17 so it can be applied cleanly after #2916781: Allow off-canvas dialog to be rendered at the top of the page.

Cleaned up a few things that weren't necessary.. I think.

Opened two issues which should allow us to remove some custom styling from this patch:

#2976287: The off-canvas dialog should have a 1px transparent border
#2976385: Provide the ability to wrap the entire page with a border when opening off-canvas in the top position

The resizing off the off-canvas is still broken; #2928531 - Off-canvas resize handlers are broken by dialog.css in Classy theme and System tray module

@Martijn de Wit, I don't see what that has to do with Workspaces using the top off-canvas dialog :) That's just a bug in the dialog's code, not in the workspace UI implementation.

Yes it isa bug in the off-canvas implementation. Not in the dialog itself. Changed the issue title.
Don't think it is blocking for the Workspaces UI. But would be very nice if you can drag the off-canvas dialog smaller or bigger.

The issues mentioned in #21 will take care of the 1px border and the "wrapping" one, so we can remove a bit more stuff from this patch :)

A few things about that close arrow...

Does it need to be an arrow? All our other dialogs and off-canvas use a cross for the close button. If workspace UI uses an arrow, but settings-tray uses an cross, the design inconsistency could be confusing.

Maybe we should consider using the arrow style for all uses of off-canvas, and change that in a follow-up? I don't have an opinion which shape is better, but it should be consistent. The arrow wouldn't make sense for closing centred dialogs though; those should continue to use a cross.

Patch #25 has a serious accessibility problem with the focus style for the close button:

+.ui-dialog.workspace-dialog.ui-dialog-off-canvas .ui-dialog-titlebar-close,
+.ui-dialog.workspace-dialog.ui-dialog-off-canvas .ui-dialog-titlebar-close:hover,
+.ui-dialog.workspace-dialog.ui-dialog-off-canvas .ui-dialog-titlebar-close:focus {
+  background-image: none;
+  border: solid;
+  border-color: transparent #ffffff #ffffff transparent;
+  border-width: 0 3px 3px 0;

In the usual cross style for dialog and off-canvas, the icon is a background-image, and a border is added to convey focus. The approach here takes over the border to create the arrow shape, and has killed the focus indicator. There must be a clear focus style, whichever shape the icon uses.

Note that we do still have the default focus style from the user agent. But we can't rely on this - the default focus styles vary considerably between user agents, and the won't always be clear enough. For instance, in Blink-based browsers, the blue outline only has a contrast ratio of 1:2 against the dark grey background in the off-canvas dialog. This doesn't satisfy WCAG SC 1.4.11 Non-text contrast:

I recommend we go back to the approach of using an image for the close icon, and use the border as the focus (and hover) style, like settings tray.

The "manage workspaces" link doesn't satisfy WCAG contrast requirements (screenshot in #27). The combination of #53b0eb blue on #444444 grey has a contrast ratio of 1:4.07. The contrast needs to be at least 1:4.5 to satisfy WCAG at level AA.

Is this is a completely new colour for workspace UI? I can't find 53b0eb in existing code, but maybe it's in another format.

We have several shades of grey background in workspace UI. I think the lightest one is #555. The blue link will also need to have sufficient contrast there too.

How about #7AD4FE for the blue links? It has a contrast of 1:4.5 against #555, and 1:5.8 against #444. I used the Tanaguru contrast finder to find this. It helps you tweak it so it's just over the threshold, to minimize disruption to existing designs.

The links will also need a focus outline which passes WCAG too. An easy way to do this is to fix the text contrast, then use outline-color: currentColor so the focus outline passes with the same ratio.

Currently links in the workspace UI dialog have no focus indicator when Seven is the main page theme, because they inherit a:focus { outline:none; } from core/themes/seven/css/base/elements.css. But when Bartik is the main page theme, links in the workspace UI have a the default style form the user agent. The page theme styles are leaking through to the workspace UI.

The yellow and green workspace icons satisfy WCAG SC 1.4.11 Non-text contrast, with a contrast ratio of 1:3.

Yellow #e09600 on #555 - 1:3.02
Yellow #e09600 on #444 - 1:3.95
Green #7dba6e on #555 - 1.3.23
Green #7dba6e on #444 - 1:4.23

Great!

The coloured workspace buttons on the toolbar don't provide enough contrast for the white text. These both fail WCAG contrast requirements for text at level AA:

• White on yellow (#e09600) - 1:2.46
• White on green (#77b259) - 1:2.53

The buttons also get a translucent gradient which further reduces the contrast, for the hover and focus states. This will need to be taken into account when improving these colours, the text needs to have 1:4.5 minimum contrast in all states.

It could be tricky finding shades of green and yellow which provide sufficient contrast against both white text and the grey dialog backgrounds.

There's a WCAG level A issue at narrow breakpoints, for SC 1.4.1 Use of Color.

At narrow breakpoints, the text labels are not visible for toolbar buttons. So a user can only tell the difference between the live and stage workspaces by colour alone. The icon is the same in each case, and the distinguishing text has been removed. Colour is the only distinguishing feature.

Live workpace active. Green is the only clue:

Stage workspace active. Yellow is the only clue:

Some users may have considerable difficulty knowing which workspace they are viewing. This impacts scenarios like:

• People with colour blindness
• People using tools which adjust the colour-space of their device (Windows/Edge high-contrast theme, iOS white-point adjustment, evening blue-light reduction, ...)
• People using their device in sunlight (e.g. reviewing changes with a phone, sitting next to the window on a train journey, on a sunny day, just before a client meeting...)

This should be addressed before marking the Workspace module as stable. As a level-A WCAG issue, I'd say it's a stable blocker. We don't have to fix it immediately here in this issue, but I'm adding it to the must-haves in the summary here.

Here's an idea of how to address the WCAG use-of-color issue in #31. Instead of removing text labels from all toolbar buttons, we keep the text label for the workspace?

In these mock-ups it's possible to tell the workspaces apart without relying on colour vision:

There's a risk the workspace name could be too long, but we could truncate it if it would be more than, say half the viewport width.

This can fixed in a follow-up issue. It might also have some utility outside of workspace module itself. We could introduce priority levels for toolbar button labels, like the responsive table column priorities we already use. It would be a hint that "this text label is really important at all breakpoints" versus "this text label can be hidden at narrow breakpoint".

We reviewed this today on a call with @yoroy and some other folks. Here are the overall notes: https://www.drupal.org/project/drupal/issues/2732071#comment-12668564

Pursuant to that, a few things we need in this patch to get it out of the "beta blocker" zone:

• Make the space usage of the toolbar more compact; since much of the functionality of the full mocks isn't implemented yet, the extra space for looks confusing. Remove the space between active workspace name and the button/links below it. [must have]
• Add a link to show list of more Workspaces (another compact tab to the right of the 4 inactive workspaces with "Show all") so there's a way to get to all workspaces. [must have]
• Remove "Status" column from overview page; relabel "Set active" to e.g. "Switch to $workspace" [must have]

Other things that were mentioned; probably best split off as follow-up issues so we don't add any more twists and turns to this one's feasibility for 8.6:

• Put the active workspace "sticky" at the top + highlighted in the overview table. [should have]
• Remember the page you were on and take you back there when switching Workspaces. [should have]
• Recent Workspaces section should be "per user"; many editors work on different parts of the site/different workspaces at the same time. [needs issue] [should have]
• Search/filter box takes up a lot of the design, given > 4 workspaces is an edge case for the vast majority of users. May need more design work. [should have]

• fixed left margin on mobile viewport when admin theme is active (see comment #26)
• aligned ‘View all workspaces’ link left
• removed customized blue link color (#7AD4FE) to now use system tray default (#85bef4)
• workspace buttons aligned to left of viewport since no search field currently
• toolbar tab link and icon changed to black as originally designed to improve contrast
• fixed padding top issues of the current workspace element on mobile
• updated green and orange icon colours

Sorry, I incorrectly outputted the patch and interdiff in #44. Please use these.

Here's an updated patch that includes workspace.overview.css.

Some comments on the CSS:

+++ b/core/modules/workspace/css/workspace.toolbar.css
@@ -12,43 +12,250 @@
+div.workspace-dialog #drupal-off-canvas .active-workspace label {
...
+div.workspace-dialog #drupal-off-canvas .active-workspace a.manage {
...
+div.workspace-dialog #drupal-off-canvas .active-workspace .actions {
...
+div.workspace-dialog #drupal-off-canvas .active-workspace .actions a.button {
...
+div.workspace-dialog #drupal-off-canvas .active-workspace .actions a.button:hover {
...
+div.workspace-dialog #drupal-off-canvas .all-workspaces {
...
+div.workspace-dialog #drupal-off-canvas .active-workspace label:before,
...
+div.workspace-dialog #drupal-off-canvas .active-workspace.default label:before,
...
+div.workspace-dialog #drupal-off-canvas .active-workspace label:before {
...
+  .toolbar .toolbar-bar .toolbar-tab > .toolbar-icon.toolbar-icon-workspace {
...
+  .toolbar .toolbar-bar .toolbar-tab > .toolbar-icon.toolbar-icon-workspace:before {
...
+  div.workspace-dialog #drupal-off-canvas .active-workspace .actions {

These seem to be overly qualifying css selectors, for instance, div.workspace-dialog #drupal-off-canvas could probably be just #drupal-off-canvas. I've not tested the patch manually, but could we simplify these? I picked these out of instict, but there might be others - also if there is a reason why we need to be so specific then never mind :)

humm isn’t qualifying with .active-workspace etc after that good enough?

Here's a review for the non-css parts:

1. +++ b/core/modules/workspace/src/WorkspaceListBuilder.php
   @@ -54,8 +61,8 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
   +    $header['id'] = $this->t('ID');
   
   @@ -65,12 +72,19 @@ public function buildHeader() {
   +      'id' => $entity->id(),
   
   @@ -107,4 +121,115 @@ public function getDefaultOperations(EntityInterface $entity) {
   +    $rows = array_slice($build['table']['#rows'], 0, 4);
   +    foreach ($rows as $row) {
   +      if ($active_workspace->id() !== $row['data']['id']) {
   +        $url = Url::fromRoute('entity.workspace.activate_form', ['workspace' => $row['data']['id']]);
   +        $default_class = $row['data']['id'] === WorkspaceInterface::DEFAULT_WORKSPACE ? 'default' : 'not-default';
   

   \Drupal\Core\Entity\EntityListBuilder::render() keys $build['table']['#rows'] by the entity ID, so we don't need to add it as an additional column.

   However, the array_slice() call will re-key the array, so we need to use a regular for () instead.

2. +++ b/core/modules/workspace/src/WorkspaceListBuilder.php
   @@ -107,4 +121,115 @@ public function getDefaultOperations(EntityInterface $entity) {
   +   * @param array $build
   ...
   +  protected function offCanvasRender(array &$build) {
   

   Needs a proper doc block :)

3. +++ b/core/modules/workspace/src/WorkspaceListBuilder.php
   @@ -107,4 +121,115 @@ public function getDefaultOperations(EntityInterface $entity) {
   +   * @throws \Drupal\Core\Entity\EntityMalformedException
   

   I don't see this thrown in the method..

4. +++ b/core/modules/workspace/src/WorkspaceListBuilder.php
   @@ -107,4 +121,115 @@ public function getDefaultOperations(EntityInterface $entity) {
   +        '#prefix' => $this->t('<span>Current workspace:</span>'),
   

   Do we really need to put the <span> element inside t(), can't we use #prefix or '#wrapper_attributes' to add a class to the wrapper element?

5. +++ b/core/modules/workspace/src/WorkspaceListBuilder.php
   @@ -107,4 +121,115 @@ public function getDefaultOperations(EntityInterface $entity) {
   +      'manage' => [
   +        '#type' => 'link',
   +        '#title' => $this->t('Manage workspaces'),
   +        '#url' => $active_workspace->toUrl('collection'),
   +        '#attributes' => [
   +          'class' => ['manage'],
   +        ],
   +      ],
   

   We now have a View all X workspaces link, so this one is redundant and can be removed.

6. +++ b/core/modules/workspace/src/WorkspaceListBuilder.php
   @@ -107,4 +121,115 @@ public function getDefaultOperations(EntityInterface $entity) {
   +    if (!$active_workspace->getRepositoryHandler() instanceof NullRepositoryHandler) {
   

   Let's play nice with #2958752: Refactor workspace content replication plugin system to three services and use !$active_workspace->isDefaultWorkspace() instead :)

7. +++ b/core/modules/workspace/src/WorkspaceListBuilder.php
   @@ -107,4 +121,115 @@ public function getDefaultOperations(EntityInterface $entity) {
   +    $build['workspaces'] = [
   +      '#theme' => 'item_list',
   +      '#items' => $items,
   +    ];
   

   This is missing the #cache parts from \Drupal\Core\Entity\EntityListBuilder::render().

Thanks @timmillwood, the PHP part of the patch looks good to me. We still need a proper frontend review, but here's a thing that stood out for me:

+++ b/core/modules/workspace/css/workspace.toolbar.css
@@ -12,43 +12,250 @@
+  height: 140px;
...
+    height: 140px;

We probably don't need this since we specify the height of the dialog in workspace_toolbar().

This is looking really good. I did notice a couple weird thing about the UX problems. Both these problems seem to happen without workspaces installed and are problems with the off-canvas dialog top. I created issues for them.
1. #2985324: The position of the toolbar moves above the off-canvas top dialog if a modal dialog is opened
2. #2985330: When the toolbar tray and the off-canvas top dialog link are both open links in the dialog are not clickable - this issue stops the user from clicking on workspace name to switch and from clicking the View all X workspaces

+++ b/core/modules/workspace/src/WorkspaceListBuilder.php
@@ -106,4 +116,121 @@ public function getDefaultOperations(EntityInterface $entity) {
+    if ($row_count > 1) {
+      $build['all_workspaces'] = [
+        '#type' => 'link',
+        '#title' => $this->t('View all @count workspaces', ['@count' => $row_count]),
+        '#url' => $active_workspace->toUrl('collection'),
+        '#attributes' => [
+          'class' => ['all-workspaces'],
+        ],
+      ];
+    }

Since the active workspace is not shown in the rows should this be > 2.

If I have only 2 workspaces, the default active 1 and the "stage" I still see "View all 2" but I can already see both.

1. +++ b/core/modules/workspace/css/workspace.overview.css
   @@ -0,0 +1,8 @@
   +/**
   + * @file
   + * Styling for the Workspace overview table.
   + */
   +
   +tr.active-workspace {
   +  background-color: #ebeae4;
   +}
   

   This should probably be located in Seven. We should add a comment to move this there before marking the module stable. Wondering also if there should be some other way to show the active workspace than just the styles.

2. +++ b/core/themes/stable/css/core/dialog/off-canvas.base.css
   @@ -4,8 +4,7 @@
   -#drupal-off-canvas *,
   -#drupal-off-canvas *:not(div) {
   +#drupal-off-canvas {
   

   This seems like a quite big change for Stable. Why is this change needed? Would it be possible to do this in a separate issue and not block this issue by that?

3. Besides this, it seems like there's still a few other things that could be improved on the CSS. Given the timeline of this issue, I will work on a patch that fixes most of the feedback I would have.

Some CSS clean-up. Will do another review based on this on Monday.

@timmillwood, we still have #60, #61 and #62.1 and .2 to address :) And @lauriii is still working on the patch as far as I understand.

#64 The problem with that selector is that it affects toolbar outside the scope of Workspaces. We probably have to find some JavaScript to override the default Toolbar behavior to fit the Workspace design, or make the off-canvas behave like Workspace expect by default.

In overall this looks good now and there isn't any beta blocking issues from my point of view.

+++ b/core/modules/workspace/css/workspace.toolbar.css
@@ -156,6 +168,7 @@
+

Can be removed on next patch or on commit whichever comes next.

#68 needs a reroll, probably due to me committing #2975334: Prevent changes that would leak into the Live workspace a few moments ago.

#68 re-rolled.

#71 seems to contain some unwanted changes, at least a missing newline at the end of workspace.overview.css - didn't look further than that, so I rerolled #68 as well and fixed #69 while I was there.

Thanks @lauriii for helping get this CSS in good shape! I think we're finally done here :)

This is looking good to me as well, code-wise, and it's also looking good after manually testing the patch. I only found a minor visual glitch that doesn't need to block commit:

+++ b/core/modules/workspace/css/workspace.overview.css
@@ -0,0 +1,9 @@
+/** @todo Move to Seven theme before Workspace is marked stable. */

Can we create a follow-up for this, if it doesn't already exist and link it to this issue? Also it seems to me that we need to create follow-up issues for most of the should-have tasks listed in #37.

We had signoff from @lauriii in #68 and all the must-haves @webchick and @yoroy pointed out are addressed, backend code looks good as well as manual testing.

Congrats to everyone putting effort into this, let's get it in!

Woohoo!

@plach, the @todo pointed out in #74 has an issue already: #2986055: Move Workspace overview list current workspace highlight color to Seven theme

As for the should-haves in #37, two of them were already implemented in the patch, and we have #2984938: Remember the page you were on and take you back there when switching Workspaces and #2983105: Recent Workspaces section should be "per user" for the other two.

Sounds good, missed the existing issues, sorry.

I almost forgot: stylelint listed a set of coding standard issues for the committed patch and couldn't autofix them all, so it would be good to open a follow-up to address them (see the attached list).

It would also be good to open a follow-up for the visual issue I found in #74, if there isn't one already.

The WCAG "Use of color" failure from #31 is still an issue. I can see Tim addressed it in #33, and the screenshot in #44 shows the fix too.

But there was a regression somewhere between #44 and the final commit, because now we're back at the problem from #31. It looks like the selector clean up in #63 broke this. The .toolbar .toolbar-icon-workspace selector is present in the final commit, but it gets over-ridden by .toolbar .toolbar-bar .toolbar-tab > .toolbar-icon in toolbar.icons.theme.css from Stable.

EDIT: the follow-up issue is #2986193: Workspace toolbar item fails WCAG Use-of-color at narrow breakpoint.

It's a must-have blocker for stability, so I'll add it to #2732071: WI: Workspace module roadmap.

Fix component following module rename.