[#28833] - [4.0] Remove useless code for alerts
- Fixed in Code Base
- 2 May 2020
- Medium
- Build: 4.0-dev
- # 28833
- Diff
- dgrammatiko:patch-1
- Pending Hound Hound is busy sniffing around... Details
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
The core.js had a conditional for the alerts checking if the alerts custom element was present and having 2 different behaviours (for custom elements or Bootstrap). The problem is that the code would never work as intended, so it's better to leave only the code for the custom element there (the conditional will never work as the custom element is always loaded there from the messages layout).
Testing Instructions
Apply the patch and run npm ci, alerts should be ok
Expected result
Actual result
Documentation Changes Required
Nope
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Repository NPM Change |
| Labels |
Added:
NPM Resource Changed
?
|
||
| Title |
|
||||||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28833.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28833.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28833.
The problem is that the code would never work as intended, so it's better to leave only the code for the custom element there (the conditional will never work as the custom element is always loaded there from the messages layout).
Can you explain why on this? I assumed this would use the bootstrap styles on people who upgrade from J3 and haven't updated a potential layout override
I assumed this would use the bootstrap styles on people who upgrade from J3 and haven't updated a potential layout override
That's not the case because the layout https://github.com/joomla/joomla-cms/blob/4.0-dev/layouts/joomla/system/message.php is automatically converted to Bootstrap 4 and also to the new alert system (the custom elements code)
The only break might happen if someone has a template with a custom override for the messages (the file in the link above) but in that case they have to override also the core.js to match their expectation (eg copy the code from the 3.x version of the core.js corresponding to renderMessages and removeMessages). Right now J4 is theoretically (in practice most probably won't work as it was never tested, eg the code was written with good intentions but without any tests is probably foobar) covering this edge case.
The only break might happen if someone has a template with a custom override for the messages (the file in the link above)
This was the scenario I had in mind with the above message (I don't actually care about people who directly edit core files).
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-05-02 10:48:50 |
| Closed_By | ⇒ | wilsonge | |
| Labels |
Added:
?
|
||
We can give it a go to reduce complexity but can always add it back if people have issues upgrading
but can always add it back if people have issues upgrading
One of the main objectives with the custom elements was to decouple from the Bootstrap (or any front end framework). If the upgrades fail then we totally missed the point. FWIW the custom elements repo iirc has examples for all the popular css frameworks just to validate our hypothesis that we're framework independent. But I guess time will tell.
Thanks
Something I noticed is the boarder around the alert when changing state of an article is missing on the right.