Originally submitted on Github
Problem/Motivation
Recent interviews and research exposed pain points around Drupal's admin experience of looking and feeling dated.
Specification
Quick overview
This image is just a quick overview for messages specs. Please use the Figma link to full specification as the main resource for specs.
Full specification
FIGMA: https://www.figma.com/file/OqWgzAluHtsOd5uwm1lubFeH/Design-system?node-i...
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 colors and sizes please login on Figma.
Remaining tasks
- Update patch styling to include time inputs
- Accessibility review
- RTL review (Right to left)
User interface changes
All messages styles will be changed, no functional differences.
Test Pages
- /admin/appearance
- /admin/modules
- /admin/modules/update (click on Check Manually and wait for result)
- /admin/reports/status (click on RUN CRON and wait for result)
Comment | File | Size | Author |
---|---|---|---|
#76 | messagesScreenshots--high-contrast.zip | 506.3 KB | huzooka |
#76 | messagesScreenshots.zip | 6.39 MB | huzooka |
#74 | interdiff.txt | 4.26 KB | lauriii |
#74 | claro-messages-3023301-74.patch | 11.63 KB | lauriii |
| |||
#73 | Screenshot 2019-09-03 at 15.20.20.png | 112.1 KB | huzooka |
Comments
Comment #2
antonellasevero CreditAttribution: antonellasevero commentedComment #3
ckrinaDesigns aren't finished yet.
Comment #4
ckrinaComment #6
saschaeggiComment #9
saschaeggiComment #10
saschaeggiThis can now be worked on! :-)
Comment #11
fhaeberleFor now, the 3 messages ('status', 'warning', 'error') are available.
What about the "todo" info message in Figma? Is that a planned one by Drupal?
Comment #12
saschaeggi@fhaeberle that's planned and not yet available in Drupal 8. But I quickly updated Figma with the inclusion of the spacings how it should be on smaller devices.
Comment #13
fhaeberleComment #14
fhaeberle@saschaeggi I added your additions for small screens.
Comment #16
ckrinaIt is great to get messages moved so fast! Thanks for everyone's work!
IMO we should take the chance to evaluate or plan the status messages implementation for Claro and/or open follow-ups if needed on core/other issues.
Edit: I've removed the @todo from Figma because the design is already agreed and it needs to be implemented too.
Comment #18
ckrinaComment #19
saschaeggi@fhaeberle can you also style the Default message? Also I did a small change to vertically center align the text if there is only 1 line of text. Should be possible to implement with flex. Thanks! (and sorry)
Comment #20
fhaeberle@saschaeggi I'll have a look.
Comment #21
fhaeberleImplemented the info style and applied vertical centering.
Comment #22
lauriiiThanks for starting to work on this! I didn't review the code yet because I wanted to first give some feedback on the designs for the design team. I think the current design looks greatest when there's a very long message in it. However, most messages are quite short; few lines on a mobile screen and a single line on desktop. I think the current design is too heavily optimized for the edge case of having a really long message.
Something else that I noticed when viewing this on an actual site was that the current designs have dramatically less weight on messages compared to what Seven used to have. As a result, messages are much harder to notice on the initial skim.
I'm also wondering if we could try to use some other color for the warning messages? It seems like the accessible color options of yellow just don't look that nice. Maybe there's another color that would look better and be still accessible.
Comment #23
huzookamessage
test module added to Clarodist Tools!Comment #24
philosurfer CreditAttribution: philosurfer commentedIs there any consideration of being able to close these messages? or is this the wrong place for that?
I think and individual close ( we would need to accommodate X on the right ) and a close all.
Comment #25
saschaeggi@philosurfer very good point. you're not the only one who misses this feature for sure! ;-)
yes this will be part of the new UI almost certainly. But it's not in the scope for the release of Claro.
Comment #26
antonellasev CreditAttribution: antonellasev as a volunteer commentedComment #27
finnsky CreditAttribution: finnsky at Skilld commentedComment #28
finnsky CreditAttribution: finnsky at Skilld commentedComment #29
finnsky CreditAttribution: finnsky at Skilld commentedComment #30
finnsky CreditAttribution: finnsky at Skilld commentedUpdated messages styles according last final specs.
1) Removed classy/messages dependency
2) Added template for messages. Added inline svg with fill property of type color. We doing modern theme here, right? :)
3) Added small .messages-list component to manage messages margin in valid BEM way.
mobile
desktop
Comment #31
eleleka CreditAttribution: eleleka at Skilld commentedDon't you think the dark design is too contrast on light theme? Also it seems less informative, I mean a small color difference between type of messages. You need to pay more attention on what type of message you've got.
Comment #32
lauriiiThe messages are dark on purpose since they are supposed to be visible. Multiple designers have reviewed this, and no one raised major concerns about this approach.
It's very hard to come up with colors that are both accessible and informative. This seemed like the best approach since it's accessible and highly visible. We've added a text to describe the priority for extra clarity.
I'd like to remind that we shouldn't feel too strongly about any particular design. We should feel OK to test if a design works with users, and if it doesn't, we shouldn't feel bad to change it to something else. We've been iterating designs for messages for months, and it would be great to move forward since we are planning to make a beta release next month.
Comment #33
fhaeberle+1 for the dark design and moving forward, I'm reviewing this.
Comment #34
jasonbarrie CreditAttribution: jasonbarrie commentedThe designs for this component have minor updates (CSS). See below for the new specs
https://www.figma.com/file/OqWgzAluHtsOd5uwm1lubFeH/Drupal-Design-system...
Comment #35
finnsky CreditAttribution: finnsky at Skilld commentedgonna apply that small change.
Comment #36
finnsky CreditAttribution: finnsky at Skilld commentedHi all. Updated with last design changes. Meet one problem that info message lost its title.
Comment #37
ckrinaHere are the final designs for Messages. To point out the Status/info message is not ready yet. But warning, error and success are ready. Also, I don't thing we need to address the Status one on this issue.
Please take into account that designs have changed from #36: Icons are new, and the paddings/margings, colors and font sizes have been integrated with the rest of the Design System.
Comment #38
lauriiiThanks @ckrina, @jasonbarrie and the whole design team for pushing the designs to the finish line!
I agree that we should leave the info message outside the scope of this patch. It's not something that is explicitly supported by core. We should open a separate issue against core to add explicit support, and after that has been resolved, we could add support in Claro.
Comment #39
finnsky CreditAttribution: finnsky at Skilld commentedRemoved everything related to messages--info (styles/tpl/icon)
Comment #40
finnsky CreditAttribution: finnsky at Skilld commentedComment #41
ant1The patch doesn't apply because of the recent changes to the pagers.
Also the focus styling of links needs to be looked at.
(old screenshot but I believe the issue still applies)
Comment #42
fhaeberle@finnsky Can you check your patch provided in #39 (#36)? It seems that there are pager styles accidentally added to the patch.
Comment #43
finnsky CreditAttribution: finnsky at Skilld commentedyep. sorry mates. gonna fix it.
Comment #44
finnsky CreditAttribution: finnsky at Skilld commentedRerolled. interdiff_36_39.txt is active
Comment #45
finnsky CreditAttribution: finnsky at Skilld commentedComment #46
finnsky CreditAttribution: finnsky at Skilld commentedFocus link imo is out of this issue area.
Comment #47
lauriiiLet's rename this file to message.css to be more consistent.
According to the design system, this should be 2px.
We can remove this since the margin-bottom is supposed to be --space-m.
Let's confirm the correct values from the design team.
Let's update icons with the most recent versions of icons.
Let's rename these to
message-list
andmessage
to be more consistent.We should add something like this to render multiple messages nicely.
Why do we need this?
We could probably remove the block. I don't think there's any reason for us to have that given that Claro isn't designed to be extended.
Comment #48
lauriiiThe padding-bottom should be 2rem according to the style guide
Comment #49
kostyashupenkoComment #50
kostyashupenkoFixed everything from #47 and #48 comments.
but:
RE: Let's confirm the correct values from the design team. (#47, 4th point)
Comment #51
finnsky CreditAttribution: finnsky at Skilld commented@kostyashupenko svg pictures has filled with type color. you should remove fill attribute in svgs.
Comment #52
ckrinaRe #41:
I'd say this might be related to some component using a white border between the component and the focus, but it doesn't makes sense using it everywhere. Specially not in dark regions.
So I'd remove the white at least on dark regions.
Comment #53
huzookaI just released cd_tools 1.8.4.
This new release adds an new message test page (at
/message/js
) that sets messages with Javascript theme functions.Comment #54
lauriiiThank you @huzooka! It seems like the JavaScript rendered messages still need some extra work.
The margins are now also defined in the style guide:
Comment #55
kostyashupenkoComment #56
kostyashupenko- Fixed icons to `path` instead of having objects with `stroke`
- Added margins between messages
Comment #57
lauriiiThank you! Setting back to needs work because the JavaScript rendered messages still need some extra work.
Comment #58
lauriiiWe would have to override at least
Drupal.theme.message
andDrupal.theme.tableDragChangedWarning
. Unfortunately, I found also multiple instances where messages class is hardcoded to UIs in PHP. 🤷♂️I think we should revert back tomessages
class (instead ofmessage
) and open a follow-up to try to remove all hardcoded instances ofmessages
class. In this case we also don't have to override these JavaScript theme functionsComment #59
lauriiiFixed #58. This looks decent on markup that hasn't been updated so we can fix those instances later in a follow-up where we standardize the way messages get rendered.
Here's how messages with old markup look like:
Comment #60
lauriiiComment #61
finnsky CreditAttribution: finnsky at Skilld commented@lauriii still not clear for me why can't we use inline svg here?
i'm not sure it is good.
Comment #62
lauriii@finnsky Sorry! I was supposed to explain that. I like the idea of using inline SVG, but I run into issues trying to implement that in JavaScript. I moved the icons to CSS to avoid that problem. Basically, we lack JavaScript APIs to create URLs for assets. 😕
Comment #63
finnsky CreditAttribution: finnsky at Skilld commentedComment #64
finnsky CreditAttribution: finnsky at Skilld commentedHi!
As described in slack added patch using https://github.com/iest/babel-plugin-inline-svg
Works pretty nice.
Pros:
1) Same source in twig and js. not duplicated in css
2) Fill with message type color
3) All nice things which give inline svg
Cons:
1) Global variable messageIcons.
Comment #65
lauriiiThis seems like a nice improvement but I'm concerned about the complexity this creates to our build processes. Until now, our JavaScript build process has been exactly the same with Drupal Core. This means that this might be the only change that would require us to have our custom build process once this gets added to core.
I'm also concerned that the library haven't been documented their license. We would at least have to work with the author to properly document what license does he wants to provide his library under.
Given these two reasons, I'd be willing to move forward with #59 or another solution that doesn't come with these downsides.
Comment #66
finnsky CreditAttribution: finnsky at Skilld commentedThis plugin is pretty simple.
It is just parse svg and inline it into js, we can do something close in /scripts/js/compile.js
This way build process will be same without extra packages.
So both downsides not looks critical.
Comment #67
huzookaReviewed #59.
The built file is missing from the patch #59.
Nice catch!
We need some high-contrast theming for these.
For IE11, we should remove the icons and the start margin of
.messages__title
and.messages__content
(IE11 does not displays SVGs in high contrast mode).On Edge, we should use e.g.
windowText
color forstroke
/fill
The bottom padding of
.messages
already determines the bottom spacing. This increases the bottom spacing to 2.5rem (40px).This is a really good question: how we want to separate the actual message items which have the same type? This Figma page suggests that we shouldn't group these items anymore.
Anyway, since
Drupal.Message
allows to insert a message anywhere, this should have amargin-top
as well imho.Comment #68
lauriiiIE 11 high contrast:
Microsoft Edge high contrast:
Comment #69
huzookaRe #68.4:
Please check the attached screenshot:
Comment #70
lauriiiThis should address #69.
Comment #71
huzookaIn review.
Comment #72
huzookaI think that these properties are for LTR.
We need to add the
/* LTR */
comment for these and handle the RTL version.Comment #73
huzookaRTL screenshot attached.
Comment #74
lauriiiThanks for the review! ✌️This should address #72.
Comment #75
huzookaI'm checking #74.
Comment #76
huzooka#74 addresses everything.
I attached the usual sc reenshots, and I will commit this in minutes.
Comment #78
huzookaThank you all! 🎉
Comment #79
svettes CreditAttribution: svettes at Acquia commentedGreat job on this, love the new look!
Comment #81
MrPaulDriver CreditAttribution: MrPaulDriver commentedI realise this issue is closed but I must say the dark theming of messages is too much.
Colored messages as suggested in #21 are much easier on the eye and no less visible or accessible.
I feel sure that many others will voice the same opinion once the theme gains wider usage.