Problem/Motivation

When previewing content on Umami the preview tools on the page are rendered without a background so that the preview information is hard to read.

Proposed resolution

Create a visual design how the preview should look like and implement that.

Remaining tasks

  • Create a design
  • Implement the design
  • Test the implementation
  • Commit šŸŽ‰

User interface changes

Issue fork drupal-3002443

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

lauriii created an issue. See original summary.

lauriii’s picture

Issue summary: View changes
kjay’s picture

Confirming that I'll add this design work into my current tasks and post here when the style guide is updated with the required details.

Version: 8.7.x-dev Ā» 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev Ā» 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev Ā» 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev Ā» 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev Ā» 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev Ā» 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev Ā» 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

lendude’s picture

Issue summary: View changes
Issue tags: +Bug Smash Initiative
StatusFileSize
new50.23 KB

Tried to reproduce this on a more current version of Drupal than the screenshot and the elements are more visible now and obscure the underlying elements even more.

Updated the screenshot to the newer version.

Version: 9.5.x-dev Ā» 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

markconroy’s picture

Priority: Normal Ā» Major

I'm going to bump this to major. What we are seeing in the screenshots is not acceptable for a demo of how great Drupal is. And I think a fix such as "let's just put a white background colour behind the preview info" would be a good start to have it acceptable. After that, maybe we could look at a fresh design for this component.

lauriii’s picture

Issue tags: +Usability

+1 that we could approach this with a quick fix first!

markconroy’s picture

Component: Umami demo Ā» CSS
Issue summary: View changes
Status: Active Ā» Needs work
StatusFileSize
new140.4 KB
new151.67 KB
new152.97 KB
new155.7 KB

Looks like this is an issue with Drupal core generally, not just with Umami.

In node.preview.css we have:

.node-preview-container {
  position: fixed;
  z-index: 499;
  box-sizing: border-box;
  width: 100%;
  padding: 10px;
}

We can remove all of that, and just leave in the padding: 10px to fix this issue.

Here's some screenshots:

Umami currently

Preview broken in Umami

Umami if we remove the position: fixed CSS
Preview working in Umami

Olivero currently
Preview broken in Olivero

Olivero if we remove the position: fixed CSS
Preview working in Olivero

In the mean time, let's remove this from the Umami issue queue and move to to the theme system.

markconroy’s picture

Status: Needs work Ā» Needs review
lauriii’s picture

I'm not sure we should be making this change this broadly. It seems like this could break some pre-existing themes. Even if it wouldn't, it would be changing the behavior in those themes. If we were to introduce that kind of change, we'd probably want to evaluate that a bit more. I think it would be easier to land a solution that fixes this in the scope of Umami. ā˜ŗļø

rkoller’s picture

StatusFileSize
new182.88 KB

i've applied the patch to a fresh 10.1.x-dev install with the demo_umami profile. the change looks straight forward and good. the only small detail i consider at least distracting on wider viewports is the position of the view mode switcher. it is shifted to the left compared to the position of the search field which is more or less in the center of the browser window. i've intentionally dragged the browser window to the maximal possible width to illustrate the offset/shift (screenshot is taken in safari 16.1 - but tested also in the latest version of firefox).
wide umami preview screen with the view mode switcher shown with an offset to the left of the center

one other detail i've noticed which is definitely out of the scope of the issue probably. but the tabbing order is sort of unexpected. if you start tabbing from the start to content editing link you get next to the select list, then the switch button. but me as a user would expect to get to the language selection (english/espanol) next but instead i tab into the admin toolbar. would i make sense to move the node preview container div down the DOM after the the toolbar-administration div?

markconroy’s picture

Component: CSS Ā» Umami demo

Thanks Lauriii. I'll move this back to Umami, get a small fix for it there, and see how we progress then for Drupal core.

markconroy’s picture

@lauriii this is ready for a review again.

lauriii’s picture

Discovered another issue which we should fix for optimal preview UX: #3342891: 'node' variable in page.html.twig is not available on preview node page.

I pushed some changes to the MR to make the preview bar slightly nicer.

The "switch" button should also not be visible when JavaScript is enabled. The button is visible because there's some CSS in Umami that overrides the .js .js-hide styles. I added !important to the display: none to avoid other CSS accidentally overriding that. We could also fix this in Umami but it feels a bit silly. That change probably shouldn't go to stable.

markconroy’s picture

Looks good to me. Will we RTBC it?

And perhaps create a follow up to see why we need to use !important. Umami should not be overriding .js-hide.

lauriii’s picture

If the changes look good for you, go for it!

I think we may want to keep the !important there because it helps prevent accidental overrides.

smustgrave’s picture

Status: Needs review Ā» Needs work
Issue tags: +Needs Review Queue Initiative
StatusFileSize
new23.58 KB

after_patch

When I apply the MR changes and this is what I see. Is it okay for the view mode to overlap the search field?

lauriii’s picture

@smustgrave did you clear caches after applying the patch?

smustgrave’s picture

Status: Needs work Ā» Reviewed & tested by the community

Maybe weekend work wasn't my friend after all. Thought all the changes took affect.

Clearing cache fixed it.

larowlan’s picture

Status: Reviewed & tested by the community Ā» Needs work

Agree, this is much better, manually tested before and after - looks good to me.

I committed this locally and was about to push when the !important in system module gave me pause.

I know it *should* be hidden, but its possible this will cause an unexpected regression for someone who (wrongly) had put some other specificity on a component that has js-hide to change the display.

E.g I could envision a scenario where someone did this

<div class="js-hide" id="something"></div>

With css

#something {
  display: block
}

But with this change their previous ID based selector would no longer win.

So can we do this without changing system js.module.css?

See https://codepen.io/larowlan/pen/zYJXZVj for a pen of the above, put the !important in and see the behaviour change.

markconroy’s picture

Status: Needs work Ā» Needs review
StatusFileSize
new280.81 KB
new300.91 KB

@lauriii @larowlan I've removed that !important. I don't think it is needed as demo_umami profile and umami (theme) does not have any override that I can see for .js .js-hide. I can't find the string .js or .js-hide anywhere inside the demo_umami directory.

Here's 2 screenshots, showing the "Switch" not visible when JS is enabled and then showing it visible when JS is disabled.

Umami preview JS enabled

Umami preview JS disabled

smustgrave’s picture

Status: Needs review Ā» Reviewed & tested by the community

Preview still looks good to me.

  • lauriii committed cc24ab06 on 10.1.x
    Issue #3002443 by markconroy, lauriii, smustgrave, larowlan: Improve...

  • lauriii committed 27e75830 on 10.0.x
    Issue #3002443 by markconroy, lauriii, smustgrave, larowlan: Improve...

  • lauriii committed 79423dc9 on 9.5.x
    Issue #3002443 by markconroy, lauriii, smustgrave, larowlan: Improve...

lauriii’s picture

Status: Reviewed & tested by the community Ā» Fixed

Committed cc24ab0 and pushed to 10.1.x. Also cherry-picked to 10.0.x and 9.5.x. Thanks!

luenemann’s picture

Version: 10.1.x-dev Ā» 9.5.x-dev

Status: Fixed Ā» Closed (fixed)

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