? ? Pending

User tests: Successful: Unsuccessful:

avatar ciar4n
ciar4n
27 Apr 2020

Pull Request for Issue #28810

Summary of Changes

Moves system alerts inline.

Styling is reverted to default which is what is been used for other inline alerts (eg. sample data plugin). If styling changes are required, this is best done upstream on the custom elements repo.

image

Testing Instructions

Apply this patch and run node build.js --compile-css to update the changed SCSS. Check alerts are displayed correctly.

Expected result

Actual result

Documentation Changes Required

avatar ciar4n ciar4n - open - 27 Apr 2020
avatar ciar4n ciar4n - change - 27 Apr 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 27 Apr 2020
Category Administration Templates (admin)
avatar zero-24
zero-24 - comment - 27 Apr 2020

Awesome thanks @ciar4n ?

avatar infograf768
infograf768 - comment - 27 Apr 2020

I have tested this item successfully on 7757a8e

Great. Simpler is better. :)


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28824.

avatar infograf768 infograf768 - test_item - 27 Apr 2020 - Tested successfully
avatar richard67
richard67 - comment - 27 Apr 2020

This is for atum, and it is great.

Could we do the same for the installation, too?

avatar zero-24
zero-24 - comment - 27 Apr 2020

I have tested this item successfully on 7757a8e

Awesome. looks much better now.

@ciar4n do you think we can make it a bit more joomla 4 style / modern from the css things? But that can be in a different PR too :)


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28824.

avatar zero-24 zero-24 - test_item - 27 Apr 2020 - Tested successfully
avatar ciar4n
ciar4n - comment - 27 Apr 2020

Could we do the same for the installation, too?

Currently it should look like this...

image

avatar ciar4n
ciar4n - comment - 27 Apr 2020

@ciar4n do you think we can make it a bit more joomla 4 style / modern from the css things? But that can be in a different PR too :)

This is best done on the custom elements repo otherwise we are styling the same element twice. This was kinda acceptable before when some alerts floated and other alerts were inline. Now that all elements are inline, styling twice makes no sense. Currently, this is here... https://github.com/joomla-projects/custom-elements/blob/master/src/scss/alert/alert.scss

avatar richard67
richard67 - comment - 27 Apr 2020

@ciar4n Yes, I just see. So all fine.

avatar infograf768 infograf768 - change - 27 Apr 2020
Status Pending Ready to Commit
avatar infograf768
infograf768 - comment - 27 Apr 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28824.

avatar ciar4n
ciar4n - comment - 27 Apr 2020

@zero-24 @infograf768 Thanks for the tests ?

avatar coolcat-creations
coolcat-creations - comment - 27 Apr 2020

Thank you @ciar4n

avatar brianteeman
brianteeman - comment - 27 Apr 2020

I am not in favour of changing the messages to be inline

avatar Bakual
Bakual - comment - 27 Apr 2020

@brianteeman Why not? Everything is better than the current floating thing.

avatar infograf768
infograf768 - comment - 27 Apr 2020

I am not in favour of changing the messages to be inline

If you are not, I am sure you have good reasons for that.
Unless you share them, your comment may have no value.

avatar dgrammatiko
dgrammatiko - comment - 27 Apr 2020

@ciar4n since the override for the alerts is just an import of the original SCSS I think you can totally remove that file ( https://github.com/joomla/joomla-cms/pull/28824/files#diff-eb1f14c7c40e114d5c452c3f49b97355 ). Unless there is some colour variables cascading there (in such case I guess you can still remove the file and push directly the colours in the Custom Elements repo)

avatar HLeithner
HLeithner - comment - 27 Apr 2020

With this be PR we are back to the old system right? which doesn't really work with more then 2 or 3 alerts/warnings at once?

avatar zero-24
zero-24 - comment - 27 Apr 2020

Well what do you mean by that? Or better what is the problem that is better in the current alert style?

It was also proposed to look into something like this: https://artemsky.github.io/vue-snotify/

Not sure wherther this would be something (ui wise) we could take a look into.

avatar ciar4n ciar4n - change - 27 Apr 2020
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 27 Apr 2020
Category Administration Templates (admin) Administration Templates (admin) JavaScript Repository
avatar ciar4n
ciar4n - comment - 27 Apr 2020

@ciar4n since the override for the alerts is just an import of the original SCSS I think you can totally remove that file

Fair point. Removed with 0d28b62

avatar dgrammatiko
dgrammatiko - comment - 27 Apr 2020

You might as well want to merge #28833

avatar brianteeman
brianteeman - comment - 27 Apr 2020

With this be PR we are back to the old system right? which doesn't really work with more then 2 or 3 alerts/warnings at once?

Exactly the problem. It is also an issue in that it moves items on the page in an unpredictable way which has accessibility issues

image

avatar coolcat-creations
coolcat-creations - comment - 27 Apr 2020

I think the stile can be changed much more space saving to avoid that.

avatar HLeithner
HLeithner - comment - 27 Apr 2020

What's with the vue-js solution suggested by tobias, simply moving the notification out of the view to a less annoying position?

avatar Bakual
Bakual - comment - 27 Apr 2020

Exactly the problem. It is also an issue in that it moves items on the page in an unpredictable way which has accessibility issues

But the current solution is even worse when having multiple and/or big alerts.
So I'd rather have this proposal here which may not be perfect but certainly better than what we have currently.

avatar richard67
richard67 - comment - 27 Apr 2020

... simply moving the notification out of the view to a less annoying position?

@HLeithner Not sure if we have any not annoying position on mobile screens.

avatar Bakual
Bakual - comment - 27 Apr 2020

What's with the vue-js solution suggested by tobias, simply moving the notification out of the view to a less annoying position?

As long as nobody proposes a PR using that one, there is not much point discussing it.

avatar dgrammatiko
dgrammatiko - comment - 27 Apr 2020

As long as nobody proposes a PR using that one, there is not much point discussing it.

You cannot possibly think to rollout alert based on VueJs...
The current code is 4 kb the VueJS version will be 140kb...
Also it's not that the current version cannot be positioned anywhere in the screen, here https://toiletkit.github.io/custom-elements/#/alert is an alternative version of the current code (by @C-Lodder me and @ciar4n ) with many possible positions...

avatar HLeithner
HLeithner - comment - 27 Apr 2020

I don't want to use the vuejs variant, this is just an example how to find a position and maybe also a time indicator if the alerts hide automatically. ( I know there is a a11y problem with this)

avatar dgrammatiko
dgrammatiko - comment - 27 Apr 2020

@HLeithner the position can be controlled by a simple attribute and the timeout is already implemented
eg:

<joomla-alert type="warning" dismiss="true" auto-dismiss="10000" position="bottom-left">
   <strong>Warning!</strong> This one will self distruct in 10secs.
</joomla-alert>

With some css like

joomla-alert[position="bottom-left"]{
 position: absolute;
 bottom: 0;
 right: 0
}

PS the css is placeholder, just to demonstrate that this is doable with minimal effort. I'm pretty sure both @ciar4n and @C-Lodder at some point had some code for this but I can't recall why we never went that way

avatar HLeithner
HLeithner - comment - 27 Apr 2020

The jumping content (when closing) is more annoying to me then a notification on the right or left bottom.

avatar ciar4n
ciar4n - comment - 27 Apr 2020

Considering I can't find one issue opened regarding the J3 alerts however this conversation keeps coming up regarding J4 alerts is reason enough to change to inline.

#16273
#25890
#13907
#16497
joomla/40-backend-template#144
joomla/40-backend-template#148

@brianteeman Going by your screenshot you need to compile the css.

moves items on the page in an unpredictable way

Presuming you are speaking about when an alert is dismissed, it is up.. always up.

There is no perfect solution but I believe users will always want to dismiss an alert if they believe that alert might be covering content regardless of where it is. All things considered, in this, simple is best.

avatar HLeithner
HLeithner - comment - 28 Apr 2020

Do you have any smarter solution for frontend alerts?

avatar ciar4n
ciar4n - comment - 28 Apr 2020

Frontend alerts have always been inline. Considering the frontend template is boxed by default, I guess they could be floated on one side.

avatar wilsonge wilsonge - change - 2 May 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-05-02 10:20:56
Closed_By wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 2 May 2020
avatar wilsonge wilsonge - merge - 2 May 2020
avatar wilsonge
wilsonge - comment - 2 May 2020

I feel like this makes the UI worse somehow but that it improves the UX - the problem is we have alerts overlapping content permanently now we removed the auto-dismiss for a11y. merging this as a first step - however feels like we could improve the styling in the future - it still feels very bootstrap 2 :/

avatar coolcat-creations
coolcat-creations - comment - 2 May 2020

Yes, I can work on the styling of the alerts @wilsonge aftter they have been merged. Thank you

Add a Comment

Login with GitHub to post a comment