Joomla! Issue Tracker | Joomla! CMS #28974 - [4.0] Template Atum - Change Alerts styling

? ?

Closed
5 Aug 2020
Medium
Build: 4.0-dev
# 28974
Diff
coolcat-creations:space-saving-alerts

Pending

Pending Hound Hound is busy sniffing around... Details

User tests: Successful: Unsuccessful:

coolcat-creations
6 May 2020

Summary of Changes

Changed the Header of the message to be next to the message to save space
Decreased paddings
Changed colors

Testing Instructions

See if you like the new style and if not, tell me what you would prefer instead.

Before:

After:

thank you @dgrammatiko for the hints where to change everything :-)

b4077a7 6 May 2020

Change Alers styling

coolcat-creations - open - 6 May 2020

coolcat-creations - change - 6 May 2020

Status

New

⇒

Pending

joomla-cms-bot - change - 6 May 2020

Category

Administration Templates (admin)

⇒

Administration Templates (admin) JavaScript Repository

coolcat-creations - comment - 6 May 2020

@dgrammatiko thank you for your fast help on this, now the PR can be tested :-)

ciar4n - comment - 6 May 2020

Should this styling extend to all alerts (e.g.. sample data plugin). If yes then would make more sense to have these changes upstream on the custom elements repo,

infograf768 - comment - 7 May 2020

Text not vertically centered in admin login page.

If Persian is chosen for admin language at admin login time, the alert is in Persian but rtl is not respected (it is normal that texts are still in English as we have no Persian installation language yet.)

Page source

<!DOCTYPE html>
<html lang="fa-ir" dir="rtl">
<head>
	<meta charset="utf-8">
	<meta name="viewport" content="width=device-width, initial-scale=1">
	<meta name="theme-color" content="#1c3d5c">
	<meta name="generator" content="Joomla! - Open Source Content Management">
	<title>general - مدیریت</title>
	<link href="/testinstall/joomla40/administrator/templates/atum/favicon.ico" rel="shortcut icon" type="image/vnd.microsoft.icon">

	<link href="/testinstall/joomla40/administrator/templates/atum/css/vendor/fontawesome-free/fontawesome.min.css?5.13.0" rel="stylesheet" />
	<link href="/testinstall/joomla40/media/vendor/skipto/css/SkipTo.css?2.1.1" rel="stylesheet" />
	<link href="/testinstall/joomla40/administrator/templates/atum/css/template-rtl.min.css?1551912b2ae894eed233f36ef01d0b05" rel="stylesheet" />
	<link href="/testinstall/joomla40/administrator/language/fa-IR/fa-IR.css?1551912b2ae894eed233f36ef01d0b05" rel="stylesheet" />
	<link href="/testinstall/joomla40/administrator/templates/atum/css/vendor/joomla-custom-elements/joomla-alert.min.css?1551912b2ae894eed233f36ef01d0b05" rel="stylesheet" />

</head>

Also I find the text a bit small:
Good example:

1rem is more legible.

infograf768 - comment - 7 May 2020

Should this styling extend to all alerts (e.g.. sample data plugin).

Looks these are fine here. Legible.

brianteeman - comment - 7 May 2020

On a wide screen with a short message the close x is a long way from the text where the eye is focused - as I type this I realise that this is true in J3 as well - still this is an opportunity to change that

ciar4n - comment - 7 May 2020

@infograf768 I'm just trying to make the point that we don't need multiple different styles of alerts. Whatever styling is chosen should be the same for all alerts unless there is good reason for otherwise.

coolcat-creations - comment - 7 May 2020

Should this styling extend to all alerts (e.g.. sample data plugin). If yes then would make more sense to have these changes upstream on the custom elements repo,

Why are not all Alerts in the same styling? Thought the backend template would be the right place? Can you give me an advise what to do? I think styling should be done by the template...

coolcat-creations - comment - 7 May 2020

@infograf768 thank you will test with RTL with the next changes too.

On a wide screen with a short message the close x is a long way from the text where the eye is focused - as I type this I realise that this is true in J3 as well - still this is an opportunity to change that

@brianteeman - How about a close text right after the message in a new line? Additionally not instead of the close icon at the top right?

brianteeman - comment - 7 May 2020

I think you will need to check that with the accessibility team

dgrammatiko - comment - 7 May 2020

I think styling should be done by the template...

It is done by the template, the file you just created affects only this template. The reasons why the css should not be inside the template.css are:

Cache busting, eg if the css is not changed there is no need to invalidate the cached file (updated BS doesn't mean that ALSO the custom elements are updated)
Joomla is modular by design, throwing everything in one file (monolithic approach) means we break the CMS' architecture

would make more sense to have these changes upstream on the custom elements repo

If you update the other repo styles it means that every template will get these styles, (eg cassiopeia and every other front end or back end template [given that they're not utilising an override]). I think leaving the default look as a close resemblance of the BS alerts will be the expected (eg most front end templates are, sadly, still based on BS)

How about a close text right after the message in a new line?

You don't need the text visible, the text exists for the screen readers but is not visible for the rest. Kinda expected behaviour

SharkyKZ - comment - 7 May 2020

Should I open an issue if I get only these alerts instead of the ugly ones?

ciar4n - comment - 7 May 2020

If you update the other repo styles it means that every template will get these styles

Fair point. In that case, I'd suggest not importing the CSS from the custom elements repo. Otherwise, you are largely styling the same element twice.

ciar4n - comment - 7 May 2020

Why are not all Alerts in the same styling?

Your CSS is only been applied to the system alerts so all other alerts are using the custom-element repo CSS.

Thought the backend template would be the right place?

Sure. I'd suggest removing the import from the custom element repo (https://github.com/joomla/joomla-cms/pull/28974/files#diff-eb1f14c7c40e114d5c452c3f49b97355R2) and instead copy it to the template. Edit as needed instead of overriding.

coolcat-creations - comment - 7 May 2020

I think styling should be done by the template...

It is done by the template, the file you just created affects only this template. The reasons why the css should not be inside the template.css are:
* Cache busting, eg if the css is not changed there is no need to invalidate the cached file (updated BS doesn't mean that ALSO the custom elements are updated)

* Joomla is modular by design, throwing everything in one file (monolithic approach) means we break the CMS' architecture

Sorry there was a misunderstanding, with "by the template" I meant inside atum, no matter if separate file or not. I think we are on the same track here :-)

would make more sense to have these changes upstream on the custom elements repo

If you update the other repo styles it means that every template will get these styles, (eg cassiopeia and every other front end or back end template [given that they're not utilising an override]). I think leaving the default look as a close resemblance of the BS alerts will be the expected (eg most front end templates are, sadly, still based on BS)

yes, agree on that, that's why I asked - I asked the question because I wondered why the styles I created where not applied to the other alerts. Since clarification from Ciaran I understand now :-)

How about a close text right after the message in a new line?

You don't need the text visible, the text exists for the screen readers but is not visible for the rest. Kinda expected behaviour

No I meant to add a close Link or Button right after the message as an alternate way to close the alert. Additionally to the cross at upper right. That's how some modals also are displayed these days...

coolcat-creations - comment - 7 May 2020

Should I open an issue if I get only these alerts instead of the ugly ones?

I don't understand your issue :-) Did you run npm after applying the patch? Anyway - give me a bit time to change this PR towards something better. Not easy but I'll try :-)

SharkyKZ - comment - 7 May 2020

I'm getting that before patch. I can't reproduce the "before" styling shown in this PR.

ciar4n - comment - 7 May 2020

@coolcat-creations Your 'before' screenshot is incorrect. You will need to run node build.js --compile-css on your 4.0-dev branch to the get correct current alert styling.

nurcihandem - comment - 3 Aug 2020

Changes are looking quite good so far, but some space might be needed between the X and the content. Centering the left headline vertically would be nice, too.

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

nurcihandem - comment - 3 Aug 2020

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

infograf768 - comment - 4 Aug 2020

@coolcat-creations
Still needs 1rem instead of .8rem
i.e. in the resulting css

#system-message-container joomla-alert div {
    padding: .3rem 2rem;
    font-size: 1rem; // change
}

to center "Message" or "Warning", etc., I suggest:

#system-message-container joomla-alert .alert-heading {
    display: flex; // change from block
    padding: .8rem;
    font-size: 1rem; // added specially for login page
    color: rgba(255, 255, 255, 0.95);
    background: var(--alert-accent-color, transparent);
    justify-content: center; // added
    align-content: center; // added
    flex-direction: column; // added
}

Give some space around the close button

#system-message-container joomla-alert div p {
      margin: .5rem .5rem .5rem 0; } // modified
    html[dir=rtl] #system-message-container joomla-alert div p {
      margin: .5rem 0 .5rem .5rem; }  // added

infograf768 - comment - 4 Aug 2020

BTW, I also found the reason why the message displayed in the login page is not similar (the alignment of the text value) to the ones in admin general.
What happens is that the system.message layout is not used. Instead is directly used /system/js/core.es6/message.es6
As that uses a span and does not include a <p>, the css is not applied correctly.

I'm pretty sure there is a simpler way than modifying the message.es6 to fit, i.e.
Change

      // Skip titles with untranslated strings
      if (typeof title !== 'undefined') {
        titleWrapper = document.createElement('span');
        titleWrapper.className = 'alert-heading';
        titleWrapper.innerHTML = Joomla.Text._(type) ? Joomla.Text._(type) : type;
        messagesBox.appendChild(titleWrapper);
      }
      
     //Add messages to the message box
      typeMessages.forEach((typeMessage) => {
        messageWrapper = document.createElement('div');
        messageWrapper.innerHTML = typeMessage;
        messagesBox.appendChild(messageWrapper);
      });

      // Skip titles with untranslated strings
      if (typeof title !== 'undefined') {
        titleWrapper = document.createElement('div');
        titleWrapper.className = 'alert-heading';
        titleWrapper.innerHTML = Joomla.Text._(type) ? Joomla.Text._(type) : type;
        messagesBox.appendChild(titleWrapper);
      }

      // Add messages to the message box
      typeMessages.forEach((typeMessage) => {
        messageWrapper = document.createElement('div');
        messageWrapper.innerHTML = '<p>' + typeMessage + '</p>';
        messagesBox.appendChild(messageWrapper);
      });

That would be for another PR but calling
@Fedik @dgrammatiko

Fedik - comment - 4 Aug 2020

As that uses a span and does not include a <p>

hm, why we have a <p> there? It not safe to have it as message container. <p> does not allows other block element inside, and if message will contain extra html markup with <div> then whole markup will be broken. And I am sure some extensions will have it.

infograf768 - comment - 4 Aug 2020

@Fedik
this the way the system.message layout is made

infograf768 - comment - 4 Aug 2020

see https://github.com/joomla/joomla-cms/blob/4.0-dev/layouts/joomla/system/message.php#L51

and except for login.php, it is that layout which is used

Fedik - comment - 4 Aug 2020

I think system.message layout need to be changed to something else than <p>

upd: it can be as in j3, a div with some class

joomla-cms/layouts/joomla/system/message.php

Lines 23 to 30 in 6e84eb7

    
           <?php if (!empty($msgs)) : ?> 
        
           	<h4 class="alert-heading"><?php echo JText::_($type); ?></h4> 
        
           	<div> 
        
           		<?php foreach ($msgs as $msg) : ?> 
        
           			<div class="alert-message"><?php echo $msg; ?></div> 
        
           		<?php endforeach; ?> 
        
           	</div> 
        
           <?php endif; ?>

infograf768 - comment - 4 Aug 2020

Ok. this means modifying both the layout and the es6 file first before a refactoring of this PR here as css have to be changed.

Fedik - comment - 4 Aug 2020

yeap, that should be easy I think

and your code for js also looks correct for <p>, for <div> it will be similar:

 // Add messages to the message box
typeMessages.forEach((typeMessage) => {
  messageWrapper = document.createElement('div');
  messageWrapper.innerHTML = `<div class="alert-message">${typeMessage}</div>`;
  messagesBox.appendChild(messageWrapper);
});

Fedik - comment - 4 Aug 2020

I think it also can be part of this PR

infograf768 - comment - 4 Aug 2020

yep. easy. will keep the div instead of the H4 in the layout

infograf768 - comment - 5 Aug 2020

@coolcat-creations
Here is a full .diff containing your PR modified to use div instead of span or p (in layout and js) as well as the modifications in the scss for ltr and rtl.

j4alerts.diff.zip

diff --git a/administrator/templates/atum/scss/_variables.scss b/administrator/templates/atum/scss/_variables.scss
index 00d006d..0228e55 100644
--- a/administrator/templates/atum/scss/_variables.scss
+++ b/administrator/templates/atum/scss/_variables.scss
@@ -166,6 +166,6 @@
 // Alerts
 $state-success-text:               var(--alert-success);
-$state-success-bg:                 lighten($green-dark, 70%);
-$state-success-border:             lighten($green-dark, 30%);
+$state-success-bg:                 lighten($green-dark, 80%);
+$state-success-border:             $green-dark;
 
 $state-info-text:                  var(--white);
diff --git a/administrator/templates/atum/scss/blocks/_alerts.scss b/administrator/templates/atum/scss/blocks/_alerts.scss
index 0c2b33e..e1c9337 100644
--- a/administrator/templates/atum/scss/blocks/_alerts.scss
+++ b/administrator/templates/atum/scss/blocks/_alerts.scss
@@ -39,5 +39,5 @@
 
 .alert-heading {
-  font-size: $font-size-lg;
+  font-size: $h4-font-size;
 }
 
@@ -53,2 +53,3 @@
   }
 }
+
diff --git a/administrator/templates/atum/scss/vendor/joomla-custom-elements/joomla-alert.scss b/administrator/templates/atum/scss/vendor/joomla-custom-elements/joomla-alert.scss
new file mode 100644
index 0000000..4504df4
--- /dev/null
+++ b/administrator/templates/atum/scss/vendor/joomla-custom-elements/joomla-alert.scss
@@ -0,0 +1,84 @@
+@import "../../variables";
+@import "../../../../../../node_modules/joomla-ui-custom-elements/dist/css/joomla-alert";
+
+// The following is a restyle for the system alerts
+
+#system-message-container joomla-alert {
+  position: relative;
+  width:100%;
+  display: flex;
+  min-width: 16rem;
+  padding: 0;
+  margin-bottom: 1rem;
+  color: var(--atum-text-dark);
+  background-color: var(--white);
+  border: 1px solid var(--alert-accent-color, transparent);
+  border-radius: .25rem;
+  transition: opacity .15s linear;
+
+  .alert-heading {
+    display: flex;
+    padding: .8rem;
+    color: rgba(255, 255, 255, 0.95);
+    background: var(--alert-accent-color, transparent);
+    justify-content: center;
+    align-content: center;
+    flex-direction: column;
+  }
+
+  .alert-link {
+    color: var(--success, inherit);
+  }
+
+  &[type="success"] {
+    --alert-accent-color: var(--success);
+  }
+
+  &[type="info"] {
+    --alert-accent-color: var(--info);
+  }
+
+  &[type="warning"] {
+    --alert-accent-color: var(--warning);
+  }
+
+  &[type="danger"] {
+    --alert-accent-color: var(--danger);
+  }
+
+  .joomla-alert--close,
+  .joomla-alert-button--close {
+    position: absolute;
+    top: 0;
+    right: .3rem;
+    font-size: 2rem;
+    color: var(--alert-accent-color);
+    background: none;
+    border: 0;
+    opacity: 1;
+
+    &:hover,
+    &:focus {
+      text-decoration: none;
+      cursor: pointer;
+      opacity: .75;
+    }
+
+    [dir=rtl] & {
+      right: auto;
+      left: 0;
+    }
+  }
+
+  div {
+    font-size: 1rem;
+    .alert-message {
+      padding: .3rem 2rem .3rem .3rem;
+      margin: .5rem;
+
+      [dir=rtl] & {
+        padding: .3rem .3rem .3rem 2rem;
+      }
+    }
+  }
+}
diff --git a/build/build-modules-js/compilecss.es6.js b/build/build-modules-js/compilecss.es6.js
index 2c15e02..c076f7f 100644
--- a/build/build-modules-js/compilecss.es6.js
+++ b/build/build-modules-js/compilecss.es6.js
@@ -53,4 +53,5 @@
           `${RootPath}/administrator/templates/atum/scss/vendor/choicesjs/choices.scss`,
           `${RootPath}/administrator/templates/atum/scss/vendor/minicolors/minicolors.scss`,
+          `${RootPath}/administrator/templates/atum/scss/vendor/joomla-custom-elements/joomla-alert.scss`,
           `${RootPath}/administrator/templates/atum/scss/vendor/joomla-custom-elements/joomla-tab.scss`,
           `${RootPath}/administrator/templates/atum/scss/vendor/fontawesome-free/fontawesome.scss`,
diff --git a/build/media_source/system/js/core.es6/message.es6 b/build/media_source/system/js/core.es6/message.es6
index f0144da..981cd42 100644
--- a/build/media_source/system/js/core.es6/message.es6
+++ b/build/media_source/system/js/core.es6/message.es6
@@ -76,5 +76,5 @@
       // Skip titles with untranslated strings
       if (typeof title !== 'undefined') {
-        titleWrapper = document.createElement('span');
+        titleWrapper = document.createElement('div');
         titleWrapper.className = 'alert-heading';
         titleWrapper.innerHTML = Joomla.Text._(type) ? Joomla.Text._(type) : type;
@@ -85,5 +85,5 @@
       typeMessages.forEach((typeMessage) => {
         messageWrapper = document.createElement('div');
-        messageWrapper.innerHTML = typeMessage;
+        messageWrapper.innerHTML = `<div class="alert-message">${typeMessage}</div>`;
         messagesBox.appendChild(messageWrapper);
       });
diff --git a/layouts/joomla/system/message.php b/layouts/joomla/system/message.php
index d102f98..a319f23 100644
--- a/layouts/joomla/system/message.php
+++ b/layouts/joomla/system/message.php
@@ -49,5 +49,5 @@
 						<div>
 							<?php foreach ($msgs as $msg) : ?>
-								<p><?php echo $msg; ?></p>
+								<div class="alert-message"><?php echo $msg; ?></div>
 							<?php endforeach; ?>
 						</div>

coolcat-creations - comment - 5 Aug 2020

@infograf768 thank you so much for your help on this. In the moment I am very occupied in teaching a class of kids and I have no clue how to merge your diff. Can you create a new PR and I will close this one? That would help me very much. Thanks a lot!!!

infograf768 - comment - 5 Aug 2020

Thanks @coolcat-creations

here is the PR to test: #30294
closing this one now.

infograf768 - change - 5 Aug 2020

Status	Pending	⇒	Closed
Closed_Date	0000-00-00 00:00:00	⇒	2020-08-05 16:48:35
Closed_By		⇒	infograf768
Labels	Added: ?

infograf768 - close - 5 Aug 2020

joomla-cms-bot - change - 5 Aug 2020

Add a Comment

Older
Newer

Joomla! Issue Tracker - CMS

[#28974] - [4.0] Template Atum - Change Alerts styling - Space saving

Summary of Changes

Testing Instructions

Add a Comment

	<?php if (!empty($msgs)) : ?>
	<h4 class="alert-heading"><?php echo JText::_($type); ?></h4>
	<div>
	<?php foreach ($msgs as $msg) : ?>
	<div class="alert-message"><?php echo $msg; ?></div>
	<?php endforeach; ?>
	</div>
	<?php endif; ?>