[#8282] - Move component message to system-message-container
- Closed
- 11 Nov 2015
- Medium
- Build: staging
- Easy Test
- # 8282
- Diff
- n9iels:component-messages
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
With the move of all the filter from the left sidebar to joomla.serachtools some component messages has a small layout issue. The messages are stick to the serachtools, without padding.
This PR moves all messages like "Enable this plugin", "This option is enabled" to a JFactory::getApplication()->enqueueMessage(). So they will be displayed above the serachtools. All messages like "There are no item available` stay below the searchtools.
Test instructions
Compare the following screenshots
1) Go to Components -> Smart Search
Before the patch:
After the patch:
2) Go to Extensions -> Manage -> Database
Before the patch:
After the patch:
3) Go to Extensions -> Install
Before patch:
After patch:
4) Go to Extensions -> update
Before patch:
After patch:
5) Go to Components -> Redirect
Before the patch:
After the patch:
6) Go to components -> Search
Before the patch:
After the patch:
7) Disable the plugin, and enable/disable the Gathering statistics options. Confirm the messages are the same before and after the patch
Question
Does anybody know if the remove of administrator/manifests/packages/pkg_weblinks.xml in this PR is something I should care about?
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
?
|
||
Looks like there are some language changes here that are unrelated
That language change is correct. The new message for com_cache has the text "Warning" by default and is generated by Factory::getApplication()->enqueueMessage(). Without that language change, it say something like:
Warning
WARNING: This can be resource intensive on sites with a large number of items!
OK
The one for the expired cache looks a bit wrong afterwards. Because the message relates to the other text. With this PR you first get the warning and only afterwards for what the warning actually is.
I would revert that as it makes more sense when the warning comes right AFTER the text it is tied to.
On a general note, I think the message generation (enqueueMessage) should be done in the view class and not in the layouts. Reason is mainly because the output isn't part of the layout anymore now and you want to show the message regardless of layout. In the view, you often also have the application object already available.
Other than that, I think it makes sense to use the API to show such warnings/notices.
Yep, looks better to me now.
I see you also moved CSS to the template. If you really want to do that, you need to do it in the LESS file (https://github.com/joomla/joomla-cms/blob/staging/administrator/templates/isis/less/template.less) and run https://github.com/joomla/joomla-cms/blob/staging/build/generatecss.php afterwards.
However I don't really see why we even have the CSS in place. The difference is only minor. Imho, we could remove the CSS and also the special class j-jed-message. Or does anyone know why that is there?
I can't find why and when that class and css were add. My only guess is that the colour is done for usability, so you can see the difference between the links and the normal text. For the line-height I have no idea...
The other css class for that view is for the loader. I think that class can stay there, because the loader is not used for many page. So we don't have to load that images every time.
And yes off course, make the changes in LESS. How could I forget that.... 
I have tested this item 
unsuccessfully on 379b513
after clicking on Smart Search, there is error shown up:
JForm::getInstance could not load file
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8282.
@datredweb I cannot reproduce that error. Also, this PR didn't change anything for JForm objects. So I think this an error on you side
| Category | ⇒ | Administration Templates (admin) UI/UX |
| Labels | |||
| Easy | No | ⇒ | Yes |
The new css in Isis
+.j-jed-message {
+ line-height: 2em;
+ color:#333333;
+}
should be added in the less file and then cli generatecss has to be used.
This PR has received new commits.
CC: @810, @datredweb
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8282.
This PR has received new commits.
CC: @810, @datredweb
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8282.
I have tested this item 
successfully on e615b73
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8282.
@n9iels It's what I had to fix in https://github.com/joomla/joomla-cms/pull/8174/files#diff-14ca7868cfa55fda9bc6a3b963e73541L398 as well :)
This PR has received new commits.
CC: @810, @datredweb, @infograf768
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8282.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-11-11 11:43:06 |
| Closed_By | ⇒ | n9iels |
This PR has received new commits.
CC: @810, @datredweb, @infograf768
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8282.
This PR has received new commits.
CC: @810, @datredweb, @infograf768
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8282.
uhm.... can anyone tell me what just happened?...
I tried to solve the merge conflicts and now it seems like I deleted all my changes for this branch 
yes. there are no changes now here. And the PR is closed. maybe you can revert the last commit and see what happens?
well, that is exactly what I did. I tried to solve the merge conflicts, but the whole staging came up with it. So I revert that commit, and this happened.
I think I have a copy of the branch on my pc (working from a laptop now). I hope so....
If you have the old state somewhere, just force push it, replacing the code on GitHub completely.
And please post here the # of the new PR
Answer
The weblinks package is required to help people who previously had weblinks as part of core to transition to the new core supported version. If weblinks doesn't exist when you update Joomla then the file is removed (https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_admin/script.php#L1435-L1443)