User tests: Successful: Unsuccessful:
Pull Request for Improvement.
Installations on backend use a layer with an animated Joomla! logo when installing something.
All except one. The install languages.
This PR adds the logo layer when installing languages too.
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Title |
|
Title |
|
note that there's the same behavior while the "Find languages" process
the image is there. i tested on chrome too. but maybe didn't had time to load.
will have to preload it. will look into it tomorrow.
should be background-image: url('../../../../media/jui/images/ajax-loader.gif');
`
but it is only working on chrome
and it should be in the php file. else it will not work for other browsers. See other views.
So remove all css changes.
Category | ⇒ | Components UI/UX |
This PR has received new commits.
CC: @MATsxm
This PR has received new commits.
CC: @MATsxm
@810 since this is used in several places i think it's better to have it the template.css and in template.js
So, made some changes:
Joomla.showLoadingDiv(TOP);
).install.languages
task (although it wouldn't be bad to have it on find languages too)Tested in Chrome 50, IE 10 (also emulated IE8 and IE9 on IE10), Opera 37 and FireFox 46 on Windows.
Also tested reducing browser width/height (simulate mobile/tablet).
Please check and comment.
I don't see the background or icon.
did you refresh the browser cache? what browser are you using?
I only see it, when I changes the height:auto
to height:100%
chrome, ff, ie,edge
but then I miss the display:none;
because now the spinner is always loaded
or when you click, find languages
no, it doesn't. please check the code and you will see that it checks what task the user is doing.
you patch:
Installing language:
ie = icon not spinning
edge = icon not spinning
chrome = icon is spinning
ff = icon is spinning
Click find - languages
No spinner at all
Installing language:
ie = icon not spinning
edge = icon not spinning
chrome = icon is spinning
ff = icon is spinning
Yes, it doesn't spin IE/Edge, but as you can see it does not spin in the loading in extensions too.
IE/Edge blocks animated gif as background-image, it seems.
Open this fiddle is IE/Edge and you'll see: https://jsfiddle.net/h9gyLcuo/1/
Click find - languages
No spinner at all
Yes, it's doing the same behaviour as the "Find updates" in extensions. I can agree that both can use this layer since they are communicating with outside servers, But let's leave that to another PR.
ok so IE/Edge animating animated gif inside pages seem to be a client parameter.
Tools -> Internet Options -> (Advanced tab) -> (Multimedia section) -> Play animations in webpages
So, IMO this PR is ok.
This PR has received new commits.
CC: @MATsxm
I have tested this item successfully on 8cae2c9
Tested on chrome desktop and android mobile
I have tested this item successfully on 8cae2c9
Works here. Is it possible to use this for other places where the logo displays instead of having multiple methods to do so?
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
|
Milestone |
Added: |
@brianteeman i thought i would never ask you this, but please remove the RTC.
I'm working on a solution that is easier to use in other views. As per @infograf768 suggestion.
Also to not be template specific so we can use it also in installation.
Status | Ready to Commit | ⇒ | Pending |
Labels |
RTC removed as requested
Labels |
Removed:
?
|
This PR has received new commits.
CC: @brianteeman, @infograf768, @MATsxm
This PR has received new commits.
CC: @brianteeman, @infograf768, @MATsxm
This PR has received new commits.
CC: @brianteeman, @infograf768, @MATsxm
Ok so i added the js to core.js and integrated the css into the js.
As a PoC added the new loading also in installation loading.
Please, apply patch, clear browser cache and:
Test should be made with Joomla supported browsers.
Sorry for the double tests, but i think, as @infograf768 said, it's better to have the same method in all cases (note this could also, in the future, be applied in joomla upload and update and package install).
Tested installing language in Extensions -> Install languages and I get the
white overlay but i dont get the logo
http://localhost/media/jui/images/ajax-loader.gif Failed to load resource:
the server responded with a status of 404 (Not Found)
This is because my site is installed in a subdirectory so you must have a
path wrong
On 13 May 2016 at 11:46, andrepereiradasilva notifications@github.com
wrote:
@Fedik https://github.com/Fedik can i also ask you to review the
vanilla JS?—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10396 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
@brianteeman the latest version doesn't have css changes, please use the latest version of this PR
commented inline on the faulty css (i think)
On 13 May 2016 at 11:55, Brian Teeman brian@teeman.net wrote:
Tested installing language in Extensions -> Install languages and I get
the white overlay but i dont get the logo
http://localhost/media/jui/images/ajax-loader.gif Failed to load
resource: the server responded with a status of 404 (Not Found)This is because my site is installed in a subdirectory so you must have a
path wrongOn 13 May 2016 at 11:46, andrepereiradasilva notifications@github.com
wrote:@Fedik https://github.com/Fedik can i also ask you to review the
vanilla JS?—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10396 (comment)Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
The image path cames from here https://github.com/joomla/joomla-cms/pull/10396/files#diff-c1b1173fc1376e9afd63e8ec801c711aR534
retested and same issue
(offline for the rest of the day now)
On 13 May 2016 at 11:58, andrepereiradasilva notifications@github.com
wrote:
The images path cames from here
https://github.com/joomla/joomla-cms/pull/10396/files#diff-c1b1173fc1376e9afd63e8ec801c711aR534—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10396 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
@andrepereiradasilva script looks good but personally I would use as second param the container instead of the positionTop and will use that to append the spinner to the given element instead of the document body. This will be handy if you want to replace the code in the joomla/extensions update
@andrepereiradasilva Also to overcome the problem Brian is pointing above use data attributes for the gif path. An idea...
@brianteeman
Hum, your error is strange. I'm using relative paths in this PR (see https://github.com/joomla/joomla-cms/pull/10396/files). Are you using a fresh latest staging?
Does anyone else have this problem?
@andrepereiradasilva script looks good but personally I would use as second param the container instead of the positionTop and will use that to append the spinner to the given element instead of the document body. This will be handy if you want to replace the code in the joomla/extensions update
Why append to an HTML element other than body? Is a layer with fixed position. Isn't the fixed position independent of it's HTML div tag position in the HTML?
@andrepereiradasilva Also to overcome the problem Brian is pointing above use data attributes for the gif path. An idea...
My goal here was to make this independent of HTML/CSS.
You can just use Joomla.loadingLayer("show");
after document ready and the layer with the logo will appear (with no image preloading in this example).
As said i'm using relative paths in this PR (see https://github.com/joomla/joomla-cms/pull/10396/files).
Please test.
This PR has received new commits.
CC: @brianteeman, @infograf768, @MATsxm
This PR has received new commits.
CC: @brianteeman, @infograf768, @MATsxm
This PR has received new commits.
CC: @brianteeman, @infograf768, @MATsxm
@andrepereiradasilva what I am proposing is something like:
elContainer = elContainer || document.getElementByTagName("BODY");
.
.
.
elContainer.innerHTML += loadingDiv;
The reason is that if we want this as a global function that will be used in install from web then in that view the spinner renders inside the relative tab and not in the entire page.
The reason is that if we want this as a global function that will be used in install from web then in that view the spinner renders inside the relative tab and not in the entire page.
Do you mean when clicking the "Add Install From Web" button?
Here https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_installer/views/install/tmpl/default.php#L18-L53
my 5 cent
My goal here was to make this independent of HTML/CSS.
I not sure that it is good idea, but in average the idea is not bad
for me it just some styling stuff, that is fine in CSS ,
that why I not very like placing this in core.js
,
maybe in cms.js
? but just a thought
I agree with @dgt41 that would be good to be able specify wich container you want to use, if you want to make it as core feature.
Additionally topPosition
should accept value in %
., and this should be applied not to top
, but to background-position-y
loadingDiv.style["background-image"] = "url('../../../media/jui/images/ajax-loader.gif')";
this part is really hard
but for now I also do not know how to make it better
I not sure that it is good idea, but in average the idea is not bad
for me it just some styling stuff, that is fine in CSS ,
The problem with that is that if you want to change it you have to change it several places (admin templates and installation template), like it is now (css, less ...).
that why I not very like placing this in core.js,
maybe in cms.js ? but just a thought
cms.js isn't loaded by neither the installation or isis.
I agree with @dgt41 that would be good to be able specify wich container you want to use, if you want to make it as core feature.
ok, will had it.
Additionally topPosition should accept value in %.,
You're right, will make it more flexible.
and this should be applied not to top, but to background-position-y
The background-position-x and background-position-y is the logo position, not the layer. Is background-position: center
to center the logo (horizontal and vertical).
The background-position-x and background-position-y is the logo position, not the layer. Is background-position: center to center the logo (horizontal and vertical).
This is confusing, at least for me. As developer I would assume that it should affect the GIF animation, not the "gif holder" itself.
why then we need topPosition
for "gif holder"?
This PR has received new commits.
CC: @brianteeman, @infograf768, @MATsxm
Changes added (allow to set parent element and other top units)
This is confusing, at least for me. As developer I would assume that it should affect the GIF animation, not the "gif holder" itself.
Added some comments to explain that better.
why then we need topPosition for "gif holder"?
This PR has received new commits.
CC: @brianteeman, @infograf768, @MATsxm
This PR has received new commits.
CC: @brianteeman, @infograf768, @MATsxm
This PR has received new commits.
CC: @brianteeman, @infograf768, @MATsxm
ready for testing
I have tested this item successfully on e6b019d
Tested in FF 46.x, Chrome 50.x and IE11, works perfect!
Tested in MS Edge 25.x, icon refuses to animate, must be due to Edge, I guess.
I have tested this item successfully on e6b019d
Seems ok to me now
This PR has received new commits.
CC: @brianteeman, @BurtNL, @infograf768, @MATsxm
Sorry guys. but you don't need to retest.
Tests are still valid as the part that was changed (missing the new arguments in js) was not used yet.
please just mark as success again
I have tested this item successfully on 2c77079
Fetched the data again and applied the patch again, still good!
I have tested this item successfully on 2c77079
with FF, Chrome and Yandex - Thanks
Status | Pending | ⇒ | Ready to Commit |
RTC - thanks
Labels |
Added:
?
|
Labels |
Added:
?
|
to all who participated in this PR. thanks!
I have tested this item unsuccessfully on 2c77079
Sorry, but this does not work here on a clean new staging install at installation time.
A fantom page displays at each page load, for example when installing sample data
Switching from one page to the other displays briefly a fantom without logo and no logo shows.
Installing languages does the same.
Thereafter, when installing a language in Extensions: install languages, I also get the fantom page and no logo.
Firefox 46.0.1 Safari Macintosh. PHP 5.4.4
Sorry I only tested the install languages from the admin page
I did not test the install languages during the installation
On 14 May 2016 at 07:56, infograf768 notifications@github.com wrote:
Also, when I set Debug on and I load the Install Languages page I get an
unresponsive script:[image: screen shot 2016-05-14 at 08 54 24]
https://cloud.githubusercontent.com/assets/869724/15266856/bcc3ee6a-19b1-11e6-9c61-cd9f800b8ed5.png—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10396 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Status | Ready to Commit | ⇒ | Pending |
Labels |
Removed the RTC
Labels |
Removed:
?
|
@brianteeman
Do you get it to work in the install languages page in back-end? Here same result as during Joomla installation.
Note: it looks like the unresponsive script is also displaying (debug on) without this patch.
Yes it worked in the admin for me - had to heavily clear the cache first
On 14 May 2016 at 09:47, infograf768 notifications@github.com wrote:
Note: it looks like the unresponsive script is also displaying (debug on)
without this patch.—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10396 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
@brianteeman
Was your test site in a sub-folder?
I didnt get it to work in a subfolder but it did work in the root
then when they said they had fixed it I moved it back to a subfolder and it
worked
On 14 May 2016 at 09:53, infograf768 notifications@github.com wrote:
@brianteeman https://github.com/brianteeman
Was your test site in a sub-folder?—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10396 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
But thinking about it maybe it was cached from when it was in the root
On 14 May 2016 at 09:58, Brian Teeman brian@teeman.net wrote:
I didnt get it to work in a subfolder but it did work in the root
then when they said they had fixed it I moved it back to a subfolder and
it workedOn 14 May 2016 at 09:53, infograf768 notifications@github.com wrote:
@brianteeman https://github.com/brianteeman
Was your test site in a sub-folder?—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10396 (comment)Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Ok, when I test with the full local url here
loadingDiv.style['background-image'] = 'url("http://localhost:8888/sqltest/trunkgitnew/media/jui/images/ajax-loader.gif")';
instead of
loadingDiv.style['background-image'] = 'url("../../../media/jui/images/ajax-loader.gif")';
I get the logo running fine, so we have an issue with the relative path.
Now, the issue during Joomla installation would not be solved, i.e the fact that at each page change we get the ghost page without any logo. For example, the ghost page should never show when we are installing sample data as we have a progress bar.
I get the logo running fine, so we have an issue with the relative path.
How? The js file is /media/system/js/core.js
(or core-uncompressed.js)
The relative path is ../../../media/jui/images/ajax-loader.gif
, so 3 folders down ../../../
is the site root, and then go back to the /media/jui/images/
folder. It seems correct to me.
Now, the issue during Joomla installation would not be solved, i.e the fact that at each page change we get the ghost page without any logo. For example, the ghost page should never show when we are installing sample data as we have a progress bar.
I think that was already like that before this PR. So, IMO, not related to this PR.
@infograf768 ok, i understand the whitescreen with no logo in installation now.
It hapened to me imeadtly after appling this patch on a clean joomla staging an acessing the url /installation/index.php?view=languages
(i didn't clean the install dir on install).
What you're experiencing it's actually the current behavior (without this patch).
The logo is there but is very down the screen (if you install several languages at the same time - like 10 languages - then you have time to scroll down and see the logo).
Actually this PR solves that issue (the logo is now centered on the viewport height, not the parent layer height) !
The thing is: you have to clear browser cache also on installation (is a different CSS).
In conclusion, there is no issue in this PR. Please check and update your test result if ok.
@andrepereiradasilva I got small idea, would be good if the method will return element
, it could be useful in future for adjust some style/position
Joomla.loadingLayer = function(task, parentElement, topPosition) {
// bla bla bla code
//...
return loadingDiv;
}
just an idea, nothing important
I got small idea, would be good if the method will return element, it could be useful in future for adjust some style/position
To use chaining?
To use chaining?
no no, I mean different, eg:
var loader = Joomla.loadingLayer("load")
loader.style.backgroundColor = '#000';
loader.style.left = '10%';
i see. good idea. will make check that (it will not change the test results so i don't see problem in changing if ok).
Here are 2 screen recordings:
Before patch
https://www.dropbox.com/s/7tz1f64aqgu2j7k/installbefore.mp4?dl=0
After patch
https://www.dropbox.com/s/fst188ild1qvc2s/installafter.mp4?dl=0
Evidently cleaned all caches, closed browser, cleaned again.
I made another test with a clean install at the root of MAMP, i.e. in htdocs.
This time the logo displays OK. The issue is definitely related to the relative path here imho.
In the current code base it shouldnt be as the link is definitely relative
now
On 15 May 2016 at 09:26, infograf768 notifications@github.com wrote:
I made another test with a clean install at the root of MAMP, i.e. in
htdocs.
This time the logo displays OK. The issue is definitely related to the
relative path here imho.—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10396 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
This time the logo displays OK. The issue is definitely related to the relative path here imho.
If so, you should have an error (404 Not Found) in de chrome console indicating the UEL the browser is trying to get the image from. Can you tell me what is that URL?
It is always looking into htdocs instead of the sub/sub/folder
"NetworkError: 404 Not Found - http://localhost:8888/media/jui/images/ajax-loader.gif"
But, as you can see in this screenshot, the js loaded is showing the relative path.
@brianteeman
You are quoting a part which is deleted by this PR as now the code is in core.js
I found a way which is really not beautiful, hoping a js expert will jump in:
if (task == 'load')
{
var loadingDiv = document.createElement('div');
var re = new RegExp(/^.*\//);
var str = re.exec(window.location.href);
var basepath = str.toString().replace('administrator/', '');
var url = "media/jui/images/ajax-loader.gif"
loadingDiv.id = 'loading-logo';
// The loading layer CSS styles are JS hardcoded so they can be used without adding CSS.
// Loading layer style and positioning.
loadingDiv.style['position'] = 'fixed';
loadingDiv.style['top'] = topPosition;
loadingDiv.style['left'] = '0';
loadingDiv.style['width'] = '100%';
loadingDiv.style['height'] = '100%';
loadingDiv.style['opacity'] = '0.8';
loadingDiv.style['filter'] = 'alpha(opacity=80)';
loadingDiv.style['overflow'] = 'hidden';
loadingDiv.style['z-index'] = '10000';
loadingDiv.style['display'] = 'none';
loadingDiv.style['background-color'] = '#fff';
// Loading logo positioning.
loadingDiv.style['background-image'] = 'url(' + basepath + url + ')';
loadingDiv.style['background-position'] = 'center';
loadingDiv.style['background-repeat'] = 'no-repeat';
loadingDiv.style['background-attachment'] = 'fixed';
parentElement.appendChild(loadingDiv);
}
i see the problem now. thanks. i will try to work on a solution.
Forgot, we would also have to replace installation/
from the basepath when it is used at install time.
And it works fine :)
This PR has received new commits.
CC: @brianteeman, @BurtNL, @infograf768, @MATsxm
ok. So i solved the relative paths.
Added and HTML data attribute (data-basepath
) to the HTML body
tag of isis and installation templates. That data attribute in read after by the js to get the site base path in
var basePath = document.getElementsByTagName('body')[0].getAttribute('data-basepath') || '';
.
You can check the difference here: 4aa65a5
Please check.
@Fedik also made the changes you said.
I have tested this item successfully on 4aa65a5
Works great for me now in the admin language installer AND in a subdirectory
@andrepereiradasilva
Can you update core.js as it has been modified in the mean while?
I tested and could also use the spinder in Hathor by using the same code.
Can you add it?
diff --git a/administrator/templates/hathor/html/com_installer/languages/default.php b/administrator/templates/hathor/html/com_installer/languages/default.php
index 09e9259..a9aa62b 100644
--- a/administrator/templates/hathor/html/com_installer/languages/default.php
+++ b/administrator/templates/hathor/html/com_installer/languages/default.php
@@ -17,5 +17,16 @@
$version = new JVersion;
-
+// Add spindle-wheel for language installation.
+JFactory::getDocument()->addScriptDeclaration('
+jQuery(document).ready(function($) {
+ Joomla.loadingLayer("load");
+ $("#adminForm").on("submit", function(e) {
+ if (document.getElementsByName("task")[0].value == "languages.install")
+ {
+ Joomla.loadingLayer("show");
+ }
+ });
+});
+');
?>
diff --git a/administrator/templates/hathor/index.php b/administrator/templates/hathor/index.php
index 7463312..505055f 100644
--- a/administrator/templates/hathor/index.php
+++ b/administrator/templates/hathor/index.php
@@ -99,5 +99,5 @@
<![endif]-->
</head>
-<body id="minwidth-body">
+<body id="minwidth-body" data-basepath="<?php echo JURI::root(true); ?>">
<div id="containerwrap">
<!-- Header Logo -->
This PR has received new commits.
CC: @brianteeman, @BurtNL, @infograf768, @MATsxm
ok done. conflicts fixed and works also on install languages on hathor now.
I have tested this item successfully on 83339ce
Works fine. @brianteeman can you confirm so that we get that in before core.js changes again?
I have tested this item successfully on 83339ce
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-05-18 07:52:50 |
Closed_By | ⇒ | rdeutz |
Labels |
Removed:
?
|
I have tested this item unsuccessfully on 39bae21
Having the white transparent background but not the logo (chrome 50.0.2661.94 m)
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10396.