[4.0] Messages/Alerts: using icons instead of text as heading #30516

infograf768 · 2020-08-30T07:40:56Z

Pull Request for Issue #30404

Summary of Changes

As title says, replacing text by icons in Alerts

Testing Instructions

Add to Atum and Cassiopea index.php

Factory::getApplication()->enqueueMessage('error message','error');
Factory::getApplication()->enqueueMessage('another long error message','error');
Factory::getApplication()->enqueueMessage('a danger message','danger');
Factory::getApplication()->enqueueMessage('warning message','warning');;
Factory::getApplication()->enqueueMessage('notice message','notice');
Factory::getApplication()->enqueueMessage('info message','info');
Factory::getApplication()->enqueueMessage('success message','success');
Factory::getApplication()->enqueueMessage('message no type');

in order to test all kinds of messages

Patch, Run NPM CI

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Cassiopea

Atum

Documentation Changes Required

brianteeman · 2020-08-30T08:32:37Z

Not got time to check properly but this looks ok for accessibility at first glance. Needs to be reviewed by the accessibility team though.

brianteeman · 2020-08-30T09:29:29Z

I have tested this item 🔴 unsuccessfully on dce6e70

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30516.}

brianteeman · 2020-08-30T09:30:55Z

After applying the patch and rebuilding npm the pr itself works great but it has a knock on impact elsewhere

before

after

infograf768 · 2020-08-30T09:37:48Z

After applying the patch and rebuilding npm the pr itself works great but it has a knock on impact elsewhere

Yep, we saw that a bit ago.
Changing to draft.

infograf768 · 2020-08-30T11:09:28Z

icomoon stuff was loaded, creating the issue with the featured columns icon.
It is now solved, but we have another issue:
The icons are very slow to load, including for all the other icons than the alerts ones.

Maybe this is due to the use of @extend in, for example

    .error,
    .danger {
      @extend .fas;
      @extend .fa-times-circle;
    }

and the fact that it is necessary if we use @extend to import fontawesome in joomla-alert.scss

We may have use a simpler

.error,
.danger {
      &::before {
        font-family: "Font Awesome 5 Free";
        font-weight: 900;
        content: "\f057";
      }

etc.

ReLater · 2020-08-30T11:37:45Z

Before this PR I had the possibility to use custom language strings for custom message types
Factory::getApplication()->enqueueMessage('message my type', 'blah');

After this PR not.

both template.scss are importing already fontawesome. Why twice with this pr?

https://github.com/joomla/joomla-cms/blob/4.0-dev/templates/cassiopeia/scss/template.scss#L15-L18

infograf768 · 2020-08-30T15:09:00Z

@ReLater

both template.scss are importing already fontawesome. Why twice with this pr?

My test with @extend created errors when compiling as @extend can only work with selectors.
To solve this I had to load again fontawesome in the file where @extend is used.
This is why I need help from a scss expert. Will test without @extend to see how it behaves.

Before this PR I had the possibility to use custom language strings for custom message types

I guess you were always getting an info type message (blue background), with blah translated in the heading.

Here we use icons related to core types.
As blah is not a specific type, it will show without a specific icon, unless you define a css for an icon in your template css.
But the text will be correctly translated.

Example:
Factory::getApplication()->enqueueMessage('message with blah as type', 'blah');

BLAH="My custom heading text"

N6REJ · 2020-08-30T21:13:28Z

This brings up an important issue I have had with the way icons are currently handled. Which me'n hans are working on. Give me 24hrs to sort this out for the best solution. One thing we don't need/want is to load fa set twice.

dgrammatiko · 2020-08-30T22:56:05Z

@infograf768 for the love of god please don't mix Fontawesome with the alerts. They are and supposed to be universal (eg no dependencies and work well with any front end stack). Just copy paste these few lines of CSS from this comment: joomla-projects/custom-elements#99 (comment)

infograf768 · 2020-08-31T06:39:24Z

@dgrammatiko
I understand it should not be done this way as anyway the icons loading time is a no no.
But I am not sure how to apply your suggestion here.

Note: let's not forget that we have 2 ways to trigger the alerts:

the layout
the js

Therefore we do need a common resulting css for both.

infograf768 · 2020-08-31T07:27:04Z

Obvioulsy, it is possible to solve the issue for the layout, using a simple code similar to the one used in brianteeman@8f6c3f9
This because we do not use the h4 anymore.

i.e. adding

$icon = [
		CMSApplication::MSG_EMERGENCY => 'fas fa-times-circle',
		CMSApplication::MSG_ALERT     => 'fas fa-times-circle',
		CMSApplication::MSG_CRITICAL  => 'fas fa-times-circle',
		CMSApplication::MSG_ERROR     => 'fas fa-times-circle',
		CMSApplication::MSG_WARNING   => 'fas fa-exclamation-triangle',
		CMSApplication::MSG_NOTICE    => 'fas fa-info-circle',
		CMSApplication::MSG_INFO      => 'fas fa-info-circle',
		CMSApplication::MSG_DEBUG     => 'fas fa-info-circle',
		'message'                     => 'fas fa-check-circle',
		'success'                     => 'fas fa-check-circle',
		'danger'                      => 'fas fa-times-circle',
];

and then

<span class="<?php echo $icon[$type]; ?>" aria-hidden="true"></span>

But the problem remains for the js as I guess it is not a good idea to hardcode there these classes.

brianteeman · 2020-08-31T07:30:50Z

That is not correct for the reasons stated in that pr when it was closed

infograf768 · 2020-08-31T08:00:11Z

I think I understood Dimitris proposal. On it now.

dgrammatiko · 2020-08-31T08:04:22Z

I think I understood Dimitris proposal. On it now.

The same approach SHOULD be applied also to all the fields. Fields should be coded in such a way that Bootstrap/jQuery or Font Awesome is not a dependency... (eg icons should be a css baground-image property if you want the CMS to remain relative in terms of performance)

infograf768 · 2020-08-31T11:37:33Z

@dgrammatiko @brianteeman
Modified to use svg. No more issue loading time.

We now get

infograf768 · 2020-08-31T11:44:17Z

Have to correct a lot of scss cs, does not prevent from testing ;)

build/media_source/system/js/core.es6/message.es6

N6REJ · 2020-08-31T15:36:44Z

why is the background size so big?

background-image:url('data:image/svg+xml;utf8,<svg width="1792" height="1792" viewBox="0 0 1792 1792" xmlns="http://www.w3.org/2000/svg"><path fill="rgba(255, 255, 255, .95)" d="M1299 813l-422 422q-19 19-45 19t-45-19l-294-294q-19-19-19-45t19-45l102-102q19-19 45-19t45 19l147 147 275-275q19-19 45-19t45 19l102 102q19 19 19 45t-19 45zm141 83q0-148-73-273t-198-198-273-73-273 73-198 198-73 273 73 273 198 198 273 73 273-73 198-198 73-273zm224 0q0 209-103 385.5t-279.5 279.5-385.5 103-385.5-103-279.5-279.5-103-385.5 103-385.5 279.5-279.5 385.5-103 385.5 103 279.5 279.5 103 385.5z"/></svg>');

Quy · 2020-08-31T15:42:27Z

infograf768 · 2020-08-31T15:50:29Z

@Quy
On it.

infograf768 · 2020-08-31T16:37:44Z

@Quy
Should be OK now.

@N6REJ

why is the background size so big?

I picked 2 of these at joomla-projects/custom-elements#99 (comment)
Then I got the others from fontawesome svg and a bit mimicked code.

layouts/joomla/system/message.php

chmst · 2020-09-01T08:40:00Z

I have tested this item ✅ successfully on c39992e

Tested on win10 with FF and Edged. Just a hint for other testers: my cache was stubborn for these changes, so using a new instance of browsers is agood idea.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30516.}

dgrammatiko · 2020-09-01T19:22:43Z

I have tested this item ✅ successfully on c39992e

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30516.}

Quy · 2020-09-01T19:24:54Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30516.}

HLeithner · 2020-09-01T20:45:58Z

Thanks

…om_templates * '4.0-dev' of github.com:joomla/joomla-cms: (70 commits) [4.0] Child templates consistency (joomla#30387) [4.0] favicon changes to support child templates (joomla#30388) [4.0] Update Readme for Api tests (joomla#30539) [4.0] [Multilingual Status module] Adding displaying a possible error if URL Language Code is empty (joomla#30537) [4.0] Display of horizontal mod_articles_news module (joomla#30527) [4.0] Useless installation lang strings (joomla#30568) [4.0] Numbers not digits (joomla#30559) [4.0] Accessibility plugin position (joomla#30552) [4.0] fix for inherit fields (joomla#30557) [4.0] Redundant words (joomla#30555) add missing legend to fieldset (joomla#30528) [4.0] [a11y] add statement on found results (joomla#30535) [4.0] com_finder ul instead of dl for easier styling (joomla#30534) [4.0] Messages/Alerts: using icons instead of text as heading (joomla#30516) [4.0] Increase API Test Coverage (joomla#26722) [4.0] Implementing display of password requirements for frontend (joomla#30473) [4.0] FieldsHelper: Choose a first available category correctly (joomla#30268) Sort options (joomla#30531) Clear checkboxes on back button (joomla#30498) Update _icomoon.scss (joomla#30436) ...

…#30516)

[4.0] Messages/Alerts: using icons instead of text as heading

dce6e70

joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Aug 30, 2020

infograf768 mentioned this pull request Aug 30, 2020

[4.0] Message width #30404

Closed

infograf768 marked this pull request as draft August 30, 2020 09:38

solving the icomoon error

f3d4a43

infograf768 added 2 commits August 31, 2020 13:20

using svg

a0e430a

white background

180a12e

infograf768 marked this pull request as ready for review August 31, 2020 11:39

dgrammatiko suggested changes Aug 31, 2020

View reviewed changes

build/media_source/system/js/core.es6/message.es6 Outdated Show resolved Hide resolved

infograf768 added 4 commits August 31, 2020 13:51

scss cs

cddd388

taking off aria-hidden in both layout and js

14ee524

cs

c07a492

tab to spaces

95283ce

infograf768 added 4 commits August 31, 2020 14:03

cs

2431732

grr

24afb2b

grr and grrr

d5f9d0e

bad robot

30907de

infograf768 added 3 commits August 31, 2020 17:54

scss cs

1f04adc

cs

d8a5966

Re-enabling heading text colour when heading is not icon

4ef5227

scss order is a pita

c39992e

Quy reviewed Aug 31, 2020

View reviewed changes

layouts/joomla/system/message.php Show resolved Hide resolved

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 1, 2020

HLeithner merged commit 6dd00a1 into joomla:4.0-dev Sep 1, 2020

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 1, 2020

HLeithner added this to the Joomla 4.0 milestone Sep 1, 2020

infograf768 deleted the 4.0_alerts_use_icons branch September 2, 2020 05:10

chmst mentioned this pull request Sep 2, 2020

Feature/4.0 dev a11y com finder refactor search result #30550

Merged

dgrammatiko mentioned this pull request Oct 16, 2020

[4.0] Improvement Vote Plugin #31098

Merged

sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020

[4.0] Messages/Alerts: using icons instead of text as heading (joomla…

a3e37e3

…#30516)

dgrammatiko mentioned this pull request Nov 27, 2020

[4.0] htmlhelper svg #31410

Closed

infograf768 mentioned this pull request Nov 29, 2020

Revert "[4.0] Webauth inline SVG button with fill" #31516

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4.0] Messages/Alerts: using icons instead of text as heading #30516

[4.0] Messages/Alerts: using icons instead of text as heading #30516

infograf768 commented Aug 30, 2020

brianteeman commented Aug 30, 2020

brianteeman commented Aug 30, 2020

brianteeman commented Aug 30, 2020

infograf768 commented Aug 30, 2020

infograf768 commented Aug 30, 2020

ReLater commented Aug 30, 2020 •

edited

infograf768 commented Aug 30, 2020

N6REJ commented Aug 30, 2020

dgrammatiko commented Aug 30, 2020

infograf768 commented Aug 31, 2020 •

edited

infograf768 commented Aug 31, 2020

brianteeman commented Aug 31, 2020

infograf768 commented Aug 31, 2020

dgrammatiko commented Aug 31, 2020

infograf768 commented Aug 31, 2020

infograf768 commented Aug 31, 2020

N6REJ commented Aug 31, 2020

Quy commented Aug 31, 2020

infograf768 commented Aug 31, 2020

infograf768 commented Aug 31, 2020

chmst commented Sep 1, 2020

dgrammatiko commented Sep 1, 2020

Quy commented Sep 1, 2020

HLeithner commented Sep 1, 2020

[4.0] Messages/Alerts: using icons instead of text as heading #30516

[4.0] Messages/Alerts: using icons instead of text as heading #30516

Conversation

infograf768 commented Aug 30, 2020

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

brianteeman commented Aug 30, 2020

brianteeman commented Aug 30, 2020

brianteeman commented Aug 30, 2020

before

after

infograf768 commented Aug 30, 2020

infograf768 commented Aug 30, 2020

ReLater commented Aug 30, 2020 • edited

infograf768 commented Aug 30, 2020

N6REJ commented Aug 30, 2020

dgrammatiko commented Aug 30, 2020

infograf768 commented Aug 31, 2020 • edited

infograf768 commented Aug 31, 2020

brianteeman commented Aug 31, 2020

infograf768 commented Aug 31, 2020

dgrammatiko commented Aug 31, 2020

infograf768 commented Aug 31, 2020

infograf768 commented Aug 31, 2020

N6REJ commented Aug 31, 2020

Quy commented Aug 31, 2020

infograf768 commented Aug 31, 2020

infograf768 commented Aug 31, 2020

chmst commented Sep 1, 2020

dgrammatiko commented Sep 1, 2020

Quy commented Sep 1, 2020

HLeithner commented Sep 1, 2020

ReLater commented Aug 30, 2020 •

edited

infograf768 commented Aug 31, 2020 •

edited