NOTE: This issue was originally about the design of the toolbar warning message, but wound up just being about the wording in the end (i.e. this issue summary is out of date). Discussion about the design has moved to #2945378: Finalize design for Toolbar warning message, and change the install screen design to match.

Problem/Motivation

This is a follow-up to "Set toolbar warning message to only appear on admin/edit pages" -#2938186: Set toolbar warning message to only appear on admin/edit pages

This issue is for what the design of the warning message is going to look like - colours/contrast/etc. Current state:

Proposed resolution

Define how to tone down

Remaining tasks

  • Define the visual changes
  • Define the desired text
  • Define the link to point to
  • Implement it (taking into account possible translations)
  • Accessibility review

User interface changes

To define

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markconroy created an issue. See original summary.

David_Rothstein’s picture

Status: Active » Needs work
FileSize
758 bytes

Is this supposed to be about the wording of the toolbar message too, or really only the visual design? (I couldn't find a separate issue about the toolbar wording, so I assume it's here.)

Here's a possible starter patch, which touches the wording only. See #2938189-9: [META] Finalise the wording of the messages in installer, toolbar and on the status report page for discussion and screenshots.

David_Rothstein’s picture

Regarding the design, the issue summary of #2938186: Set toolbar warning message to only appear on admin/edit pages says:

The permanent message warning tone is a bit aggressive. Let's calm it down a bit.

I completely agree, and I would just like to point out that the most obvious way to make it less aggressive is to get rid of the yellow exclamation-point warning icon....

This message is not really a warning condition - rather, it is informational in nature. Designing it to look like an informational message would make it way less distracting, so I suggest considering that. Drupal even has an "i" information icon (or at least used to, in Drupal 7?) which could be used instead of the warning icon.

ckrina’s picture

Thank you @David_Rothstein, indeed this is for the wording too.

As discussed in the last Out of the Box call, I'm uploading an screenshot of the Toolbar warning message with a link to take into account more possible solutions. The idea of the link is not trying to solve all we need to communicate in one phrase.

Another thing that we should take into account is the length of the messages because it can be really long in translations.

Also adding @andrewmacpherson accessibility suggestions here from the previous issue:

1. Colour contrast.
The colour of the message is #78520d on #fdf8ed, which has a contrast ratio of 6.6:1, which satisifies WCAG at level AA.

However we aren't using this colour combination anywhere else in Drupal core. The existing style for warning messages in Seven and Classy is slightly stronger: #734c00 on #fdf8ed. This has a contrast ratio of 7.2:1 which satisfies WCAG at level AAA. Can we re-use this combination please?

Adding another issue that @markconroy found in the meta issue:

Big issue: Clicking on this message still takes you to https://www.drupal.org/project/drupal/issues/2829101 - that doesn't make sense. Still needs work to figure out a good page to link to.

In the OOTB channel it was suggested to have an Out of The Box Guide in the Documentation, so maybe it would be a good place to link.

ckrina’s picture

Issue summary: View changes
ckrina’s picture

Issue summary: View changes
FileSize
93.54 KB

We discussed this in the UX meeting and @benfisher suggested to use hook_help for the link instead of a Drupal.org issue. He mentioned that it would help us to have it updated too, compared with the suggestion of creating a Guide in documentation and linking it.
@roland.molnar suggested this code as an example:

function standard_help($route_name, \Drupal\Core\Routing\RouteMatchInterface $route_match) {
  switch ($route_name) {
    case ‘help.page.standard’:
      $output = ‘It. Works.’;
      return $output;
  }
}

Here's his screenshot testing it:

About the visual changes, it hasn't been an detailed list of changes to do, just the need to calm it down.

David_Rothstein’s picture

We discussed this in the UX meeting and @benfisher suggested to use hook_help for the link instead of a Drupal.org issue.

I actually had a similar thought (see #2822414-61: Redesign the 'install profile selection' installer screen to allow for experimental profiles and more information, and that issue has some already-written text to use as a starting point). It is good to know that it actually works!

Presumably, if the Help module is disabled (or if the current user doesn't have access to it) then the toolbar message just wouldn't be shown at all?

ckrina’s picture

Oh, good point! This behaviour needs to be defined then.

I wouldn't remove the warning message or even the option to have a link, but no idea if this is even possible to override it when the help is enabled...

markconroy’s picture

My preference would be for it to link to an internal Umami page (presumably using the Basic Page content type). Then we'd know it's always going to be there, whether Help module is enabled or not.

Also, it would give another example content page. I think we've only got on Basic Page node at the moment!

ckrina’s picture

@markconroy right, that would help us avoiding to check if help is activated or not. +1 to that. I guess it should be added in this issue?

markconroy’s picture

As a design, I quite like what you have in #4 Ckrina - short sentence with the blue link.

David_Rothstein’s picture

Someone could still delete the Basic Page though. I actually think that's more likely than turning off the Help module.

Having it depend on the Help module sort of made sense to me because it's essentially help text... However, I guess another option could to be to have it go to an entirely custom page defined via Drupal's routing system (i.e. not a piece of Basic Page content, but something that isn't a node and which you can't delete). However, that would be a bit outside the way Drupal modules usually provide this kind of information.

markconroy’s picture

Would the safest be to just link to a documentation page on Drupal.org?

David_Rothstein’s picture

I suppose it would.

But it would be a good idea for this profile to have some help text anyway (and the normal pattern is basically that the help text is what links off to drupal.org for more in-depth information).

I could see it going either way... but don't think it would be that big of a deal if this message just disappeared for people who turned off the Help module. If they don't want helpful messages, then they don't have to see them :)

ckrina’s picture

So we are blocked on this decision: where should we link the warning message from the toolbar?", and options are:

  1. hook_help. But what happens if help is disabled?
  2. Default content page. It has to be written if we do so.
  3. d.o page, but random page or Documentation?

My vote would go for default content page so we don't depend from external things, and on this internal page link to Documentation on Drupal.org when we have a guide. And for now we can just explain what we need to say in the internal page.

Any other opinion?

markconroy’s picture

I agree with @ckrina on this. I'd much prefer to have this as an internal page.

I propose we set the path to /experimental and then we can have this issue fixed, while we focus on the content.

markconroy’s picture

Status: Needs work » Needs review
FileSize
706 bytes

Patch attached to set path to /experimental

yoroy’s picture

Yes lets go for an internal page, that's the best way forward for now. Can always link to a docs page from inside its content, eventually.

ckrina’s picture

Status: Needs review » Reviewed & tested by the community

It would be great that someone tests it also, but for me it's working fine. I've just created a follow-up for the internal page: #2941582: Create a drupal.org documentation page for Umami profile to explain why it shouldn't be used in production sites.

The last submitted patch, 2: 2938800-2.patch, failed testing. View results

larowlan’s picture

Status: Reviewed & tested by the community » Postponed

I think this should be postponed on #2941582: Create a drupal.org documentation page for Umami profile to explain why it shouldn't be used in production sites, we can't link to a page that doesn't exist.

Or alternatively, incorporated over there, with review credits applied from here to there.

And then close this as duplicate

David_Rothstein’s picture

Title: Finalise design for Toolbar warning message » Finalize design and wording for Toolbar warning message
Status: Postponed » Needs work
FileSize
819 bytes

That issue was committed, so this can be un-postponed now.

However, it definitely shouldn't be closed as a duplicate, since fixing the link wasn't the only thing this issue was about. (Whether this issue still needs the "Umami beta blocker" tag, though, is another question - from my point of view the remaining work here is definitely a stable-release blocker, but maybe not a beta blocker.)

Here is a reroll of the patch from #2 to apply cleanly now that the other issue has been committed. Still leaving it as needs work, since the patch only addresses the wording, but not the design (although those could probably be split into two issues if necessary).

larowlan’s picture

Fair enough, agree on the beta blocking status however. Changing the tag

smaz’s picture

FileSize
20.37 KB

I've tested the latest patch, this is ok in terms of warning text:

Toolbar warning screenshot

We seem to have lost track on the design for this: Are we happy with the current design?

If so, I'd consider this RTBC - we've discussed the text on our weekly initiative call, and the patch is fine & applies ok.

If this needs design changes, can we have a clear guidance on this?

smaz’s picture

Status: Needs work » Needs review

Needs review as to if we're happy with where we're up to or not.

smaz’s picture

Also, a screenshot from hover.

Umami warning link on hover.

Status: Needs review » Needs work

The last submitted patch, 22: 2938800-22.patch, failed testing. View results

markconroy’s picture

Status: Needs work » Needs review
FileSize
2.87 KB
1.91 KB

Here's a new patch that should fix the failing tests (testing the wrong string of text).

cehfisher’s picture

FileSize
57.01 KB

Looks like SimplyTest.me is still not working with 8.6.x, but locally the patch applied cleanly.

markconroy’s picture

During our weekly call yesterday we discussed the scope of this issue and decided that the design was not changing, only the content.

With that in mind, I think this is ready for RTBC.

Eli-T’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
24.04 KB
26.46 KB

Installed 8.6.x with patch in #28.

Warning appears as follows:

Warning tested to be present on /admin/content, /node/1/edit?destination=/admin/content, /node/add, /node/add/recipe, /admin/people

Warning tested to be not present on /, /recipes/super-easy-vegetarian-pasta-bake, /articles.

Warning has the following appearance on focus and on hover

Therefore moving this to RTBC

(comment edited to fix the path to the second embedded image)

navneet0693’s picture

FileSize
173.49 KB

There it is ! Tested on re-installed profile with patch in #28.

toolbar warning screenshot on admin/content page

EDIT: Oopss! didn't scrolled up / refreshed the page, it was already in RTBC.

quicksketch’s picture

  • Gábor Hojtsy committed d1a04fb on 8.6.x
    Issue #2938800 by markconroy, David_Rothstein, ckrina, smaz, Eli-T,...

  • Gábor Hojtsy committed 1285938 on 8.5.x
    Issue #2938800 by markconroy, David_Rothstein, ckrina, smaz, Eli-T,...
Gábor Hojtsy’s picture

Title: Finalize design and wording for Toolbar warning message » Finalize wording for Toolbar warning message
Version: 8.6.x-dev » 8.5.x-dev
Status: Reviewed & tested by the community » Fixed

Thanks all! Great seeing stable blockers resolved :)

David_Rothstein’s picture

Issue summary: View changes
Issue tags: -Umami stable blocker

Splitting it up makes sense, but the reason I proposed that this be a stable release blocker (and presumably the reason @larowlan went ahead and added that tag?) definitely wasn't the minor wording tweak - it was to improve the design. But that is a bit larger scope, so it is good to split it up.

I went ahead and created #2945378: Finalize design for Toolbar warning message, and change the install screen design to match as a child issue now, and transferred the "Umami stable blocker" tag there.

Status: Fixed » Closed (fixed)

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