Comments

mherchel created an issue. See original summary.

mherchel’s picture

Assigned: mherchel » Unassigned
kostyashupenko’s picture

Status: Active » Needs review
StatusFileSize
new16.67 KB
mherchel’s picture

StatusFileSize
new16.56 KB

Reroll attached

mherchel’s picture

I'm not too familiar with the Drupal.theme, so I might not be the best to review it, or test it. From what I can tell everything looks to be working as expected.

The one concern that I have is that we're duplicating the SVG code from status-messages.html.twig. It might make sense to move the SVGs to external files, and reference them with img elements, so we can be sure that the code doesn't diverge in the future.

dunebl’s picture

Status: Needs review » Needs work

3 small tests on the messages... I hope it is inline with what is asked:

test1: If I clear the caches, I can see a message (cache cleared) with a closing button=>OK
test2: If I add an error in the code $form['this_key_do_not_exists']='error'; I can see a message without closing button and with the text which extends outside the dark background (Impossible to read it as it is white on white) => NOT OK
test3: If I create an element validation error (Ex: upload an xls in a widget asking for jpg) then there are NO message, which is really bad => NOT OK => see #3157576: messages color not set when error on file upload for more info on test3

mherchel’s picture

Title: Override Drupal.theme.message too to make sure JS messages get rendered correctly » Override Drupal.theme.message to to make sure JS messages get rendered correctly
kostyashupenko’s picture

Status: Needs work » Needs review
StatusFileSize
new18.8 KB
new2.05 KB
new91.71 KB

test2: If I add an error in the code $form['this_key_do_not_exists']='error'; I can see a message without closing button and with the text which extends outside the dark background (Impossible to read it as it is white on white) => NOT OK

Can you provide more clear steps and explanations about what and where and how to reproduce?

test3: If I create an element validation error (Ex: upload an xls in a widget asking for jpg) then there are NO message, which is really bad

Should be fine now. I decided to not override file.js to change messages markup. So let's keep messages for file widget without close button. Check the following styles below, this is how error messages looks right now for file widget:

file messages

kostyashupenko’s picture

StatusFileSize
new18.95 KB
new3.38 KB

Reroll

mherchel’s picture

Status: Needs review » Reviewed & tested by the community

Tested through all messages that I could come up with, and went through the code.

I'm not too familiar with Drupal.theme(), but from what I can tell, this is perfect. If there are unresolved bugs, we can catch them in followup issues.

mherchel’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thank you!

Status: Fixed » Closed (fixed)

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

anybody’s picture