? ? Pending

User tests: Successful: Unsuccessful:

avatar andrepereiradasilva
andrepereiradasilva
10 May 2016

Pull Request for Improvement.

Summary of Changes

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.

image

Testing Instructions

  1. Use latest staging, apply patch and clean browser cache
  2. Go to Extensions -> Manage -> Install Languages
  3. Install one or several languages.
    You will now see joomla logo when installing the languages packages.
avatar andrepereiradasilva andrepereiradasilva - open - 10 May 2016
avatar andrepereiradasilva andrepereiradasilva - change - 10 May 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 10 May 2016
Labels Added: ?
avatar andrepereiradasilva andrepereiradasilva - change - 10 May 2016
The description was changed
avatar andrepereiradasilva andrepereiradasilva - change - 10 May 2016
Title
[Install Languages] Add logo layer on install languages
[Install Languages] Add logo layer on install languages packages
avatar andrepereiradasilva andrepereiradasilva - change - 10 May 2016
Title
[Install Languages] Add logo layer on install languages
[Install Languages] Add logo layer on install languages packages
avatar MATsxm MATsxm - test_item - 11 May 2016 - Tested unsuccessfully
avatar MATsxm
MATsxm - comment - 11 May 2016

I have tested this item :red_circle: unsuccessfully on 39bae21

Having the white transparent background but not the logo (chrome 50.0.2661.94 m)

lginstall


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

avatar MATsxm
MATsxm - comment - 11 May 2016

note that there's the same behavior while the "Find languages" process


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

avatar andrepereiradasilva
andrepereiradasilva - comment - 11 May 2016

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.

avatar 810
810 - comment - 11 May 2016

should be background-image: url('../../../../media/jui/images/ajax-loader.gif');`
but it is only working on chrome

avatar andrepereiradasilva
andrepereiradasilva - comment - 11 May 2016

of course @810 relative path forgot about that

avatar 810
810 - comment - 11 May 2016

and it should be in the php file. else it will not work for other browsers. See other views.

So remove all css changes.

avatar brianteeman brianteeman - change - 11 May 2016
Category Components UI/UX
avatar joomla-cms-bot
joomla-cms-bot - comment - 11 May 2016

This PR has received new commits.

CC: @MATsxm


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 11 May 2016

This PR has received new commits.

CC: @MATsxm


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

avatar andrepereiradasilva
andrepereiradasilva - comment - 11 May 2016

@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:

  • Moved almost js to template.js and converted code to vannila JS (except the events part)
  • Now the layer is full size, i.e, top =0 (works better on mobile), but that can be changed by dynamicly defining the top (Joomla.showLoadingDiv(TOP);).
  • The layer only appears on install.languages task (although it wouldn't be bad to have it on find languages too)
  • Since the layer is added to DOM before the submit event the image is now preloaded

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.

avatar 810
810 - comment - 11 May 2016

I don't see the background or icon.

avatar andrepereiradasilva
andrepereiradasilva - comment - 11 May 2016

did you refresh the browser cache? what browser are you using?

avatar 810
810 - comment - 11 May 2016

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

avatar andrepereiradasilva
andrepereiradasilva - comment - 11 May 2016

@810 the layer and the spinner is only showed to user when he install a language.

avatar 810
810 - comment - 11 May 2016

or when you click, find languages

avatar andrepereiradasilva
andrepereiradasilva - comment - 11 May 2016

no, it doesn't. please check the code and you will see that it checks what task the user is doing.

avatar andrepereiradasilva andrepereiradasilva - change - 11 May 2016
The description was changed
avatar 810
810 - comment - 11 May 2016

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

avatar andrepereiradasilva
andrepereiradasilva - comment - 11 May 2016

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.

avatar andrepereiradasilva andrepereiradasilva - change - 11 May 2016
The description was changed
avatar andrepereiradasilva
andrepereiradasilva - comment - 11 May 2016

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.

avatar joomla-cms-bot
joomla-cms-bot - comment - 11 May 2016

This PR has received new commits.

CC: @MATsxm


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

avatar brianteeman brianteeman - test_item - 11 May 2016 - Tested successfully
avatar brianteeman
brianteeman - comment - 11 May 2016

I have tested this item :white_check_mark: successfully on 8cae2c9

Tested on chrome desktop and android mobile


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

avatar infograf768 infograf768 - test_item - 13 May 2016 - Tested successfully
avatar infograf768
infograf768 - comment - 13 May 2016

I have tested this item :white_check_mark: 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?


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

avatar brianteeman brianteeman - change - 13 May 2016
Status Pending Ready to Commit
avatar brianteeman
brianteeman - comment - 13 May 2016

RTC


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

avatar joomla-cms-bot joomla-cms-bot - change - 13 May 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 13 May 2016
Milestone Added:
avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

@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.

avatar brianteeman brianteeman - change - 13 May 2016
Status Ready to Commit Pending
Labels
avatar brianteeman
brianteeman - comment - 13 May 2016

RTC removed as requested


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

avatar joomla-cms-bot joomla-cms-bot - change - 13 May 2016
Labels Removed: ?
avatar joomla-cms-bot
joomla-cms-bot - comment - 13 May 2016

This PR has received new commits.

CC: @brianteeman, @infograf768, @MATsxm


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 13 May 2016

This PR has received new commits.

CC: @brianteeman, @infograf768, @MATsxm


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 13 May 2016

This PR has received new commits.

CC: @brianteeman, @infograf768, @MATsxm


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

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

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).

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

@Fedik can i also ask you to review the vanilla JS?

avatar brianteeman
brianteeman - comment - 13 May 2016

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/

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

@brianteeman the latest version doesn't have css changes, please use the latest version of this PR

avatar brianteeman
brianteeman - comment - 13 May 2016

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 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/

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016
avatar brianteeman
brianteeman - comment - 13 May 2016

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/

avatar dgt41
dgt41 - comment - 13 May 2016

@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

avatar dgt41
dgt41 - comment - 13 May 2016

@andrepereiradasilva Also to overcome the problem Brian is pointing above use data attributes for the gif path. An idea...

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

@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?

@dgt41

@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.

avatar joomla-cms-bot
joomla-cms-bot - comment - 13 May 2016

This PR has received new commits.

CC: @brianteeman, @infograf768, @MATsxm


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 13 May 2016

This PR has received new commits.

CC: @brianteeman, @infograf768, @MATsxm


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

cb35879 13 May 2016 avatar andrepereiradasilva cs
avatar joomla-cms-bot
joomla-cms-bot - comment - 13 May 2016

This PR has received new commits.

CC: @brianteeman, @infograf768, @MATsxm


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

avatar dgt41
dgt41 - comment - 13 May 2016

@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.

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

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

avatar Fedik
Fedik - comment - 13 May 2016

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 :smile:
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 :wink:
but for now I also do not know how to make it better

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

I not sure that it is good idea, but in average the idea is not bad :smile:
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).

avatar Fedik
Fedik - comment - 13 May 2016

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"? :wink:

avatar joomla-cms-bot
joomla-cms-bot - comment - 13 May 2016

This PR has received new commits.

CC: @brianteeman, @infograf768, @MATsxm


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

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

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"? :wink:

See https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_installer/views/install/tmpl/default.php#L18-L53

avatar joomla-cms-bot
joomla-cms-bot - comment - 13 May 2016

This PR has received new commits.

CC: @brianteeman, @infograf768, @MATsxm


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 13 May 2016

This PR has received new commits.

CC: @brianteeman, @infograf768, @MATsxm


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 13 May 2016

This PR has received new commits.

CC: @brianteeman, @infograf768, @MATsxm


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

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

ready for testing

avatar BurtNL BurtNL - test_item - 13 May 2016 - Tested successfully
avatar BurtNL
BurtNL - comment - 13 May 2016

I have tested this item :white_check_mark: 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.


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

avatar brianteeman brianteeman - test_item - 13 May 2016 - Tested successfully
avatar brianteeman
brianteeman - comment - 13 May 2016

I have tested this item :white_check_mark: successfully on e6b019d


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

avatar brianteeman
brianteeman - comment - 13 May 2016

Seems ok to me now


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 13 May 2016

This PR has received new commits.

CC: @brianteeman, @BurtNL, @infograf768, @MATsxm


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

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

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.

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

please just mark as success again

avatar BurtNL BurtNL - test_item - 13 May 2016 - Tested successfully
avatar BurtNL
BurtNL - comment - 13 May 2016

I have tested this item :white_check_mark: successfully on 2c77079

Fetched the data again and applied the patch again, still good!


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

avatar MATsxm MATsxm - test_item - 13 May 2016 - Tested successfully
avatar MATsxm
MATsxm - comment - 13 May 2016

I have tested this item :white_check_mark: successfully on 2c77079

:+1: with FF, Chrome and Yandex - Thanks


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

avatar brianteeman brianteeman - change - 13 May 2016
Status Pending Ready to Commit
avatar brianteeman
brianteeman - comment - 13 May 2016

RTC - thanks


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

avatar joomla-cms-bot joomla-cms-bot - change - 13 May 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 13 May 2016
Labels Added: ?
avatar andrepereiradasilva
andrepereiradasilva - comment - 13 May 2016

to all who participated in this PR. thanks!

avatar infograf768 infograf768 - test_item - 14 May 2016 - Tested unsuccessfully
avatar infograf768
infograf768 - comment - 14 May 2016

I have tested this item :red_circle: unsuccessfully on 2c77079


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

avatar infograf768
infograf768 - comment - 14 May 2016

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
screen shot 2016-05-14 at 07 57 48

Switching from one page to the other displays briefly a fantom without logo and no logo shows.
Installing languages does the same.

screen shot 2016-05-14 at 08 14 11

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

avatar infograf768
infograf768 - comment - 14 May 2016

Also, when I set Debug on and I load the Install Languages page I get an unresponsive script:

screen shot 2016-05-14 at 08 54 24

avatar brianteeman
brianteeman - comment - 14 May 2016

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/

avatar brianteeman brianteeman - change - 14 May 2016
Status Ready to Commit Pending
Labels
avatar brianteeman
brianteeman - comment - 14 May 2016

Removed the RTC


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

avatar joomla-cms-bot joomla-cms-bot - change - 14 May 2016
Labels Removed: ?
avatar infograf768
infograf768 - comment - 14 May 2016

@brianteeman
Do you get it to work in the install languages page in back-end? Here same result as during Joomla installation.

avatar infograf768
infograf768 - comment - 14 May 2016

Note: it looks like the unresponsive script is also displaying (debug on) without this patch.

avatar brianteeman
brianteeman - comment - 14 May 2016

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/

avatar infograf768
infograf768 - comment - 14 May 2016

@brianteeman
Was your test site in a sub-folder?

avatar brianteeman
brianteeman - comment - 14 May 2016

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/

avatar brianteeman
brianteeman - comment - 14 May 2016

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 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/

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar infograf768
infograf768 - comment - 14 May 2016

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.

avatar andrepereiradasilva
andrepereiradasilva - comment - 14 May 2016

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.

avatar andrepereiradasilva
andrepereiradasilva - comment - 14 May 2016

@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.

avatar Fedik
Fedik - comment - 14 May 2016

@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

avatar andrepereiradasilva
andrepereiradasilva - comment - 14 May 2016

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?

avatar Fedik
Fedik - comment - 14 May 2016

To use chaining?

no no, I mean different, eg:

var loader = Joomla.loadingLayer("load")
loader.style.backgroundColor = '#000';
loader.style.left = '10%';
avatar andrepereiradasilva
andrepereiradasilva - comment - 14 May 2016

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).

avatar andrepereiradasilva
andrepereiradasilva - comment - 14 May 2016

@Fedik if you don't mind i will add it when if this PR is merged. Already add to many changes in this.
I will also remove the topPosition that becomes useless after that change.

avatar infograf768
infograf768 - comment - 15 May 2016

@andrepereiradasilva

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.

avatar infograf768
infograf768 - comment - 15 May 2016

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.

avatar brianteeman
brianteeman - comment - 15 May 2016

In the current code base it shouldnt be as the link is definitely relative
now

  • background: url('../../../media/jui/images/ajax-loader.gif') rgba( 255,255,255,0.7) center no-repeat;

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/

avatar andrepereiradasilva
andrepereiradasilva - comment - 15 May 2016

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?

avatar infograf768
infograf768 - comment - 16 May 2016

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.

screen shot 2016-05-16 at 08 51 32

@brianteeman
You are quoting a part which is deleted by this PR as now the code is in core.js

avatar infograf768
infograf768 - comment - 16 May 2016

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);
        }
avatar andrepereiradasilva
andrepereiradasilva - comment - 16 May 2016

i see the problem now. thanks. i will try to work on a solution.

avatar infograf768
infograf768 - comment - 16 May 2016

Forgot, we would also have to replace installation/ from the basepath when it is used at install time.

avatar infograf768
infograf768 - comment - 16 May 2016

And it works fine :)

avatar joomla-cms-bot
joomla-cms-bot - comment - 16 May 2016

This PR has received new commits.

CC: @brianteeman, @BurtNL, @infograf768, @MATsxm


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

avatar andrepereiradasilva
andrepereiradasilva - comment - 16 May 2016

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.

avatar brianteeman brianteeman - test_item - 16 May 2016 - Tested successfully
avatar brianteeman
brianteeman - comment - 16 May 2016

I have tested this item :white_check_mark: successfully on 4aa65a5

Works great for me now in the admin language installer AND in a subdirectory


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

avatar infograf768
infograf768 - comment - 17 May 2016

@andrepereiradasilva
Can you update core.js as it has been modified in the mean while?

avatar infograf768
infograf768 - comment - 17 May 2016

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 -->
avatar andrepereiradasilva
andrepereiradasilva - comment - 17 May 2016

will solve the conflicts, do the hathor changes and update the code comment block (forgot to remove the topposition variable) after #10521 gets merged.

avatar joomla-cms-bot
joomla-cms-bot - comment - 17 May 2016

This PR has received new commits.

CC: @brianteeman, @BurtNL, @infograf768, @MATsxm


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

avatar andrepereiradasilva
andrepereiradasilva - comment - 17 May 2016

ok done. conflicts fixed and works also on install languages on hathor now.

avatar infograf768 infograf768 - test_item - 18 May 2016 - Tested successfully
avatar infograf768
infograf768 - comment - 18 May 2016

I have tested this item :white_check_mark: successfully on 83339ce

Works fine. @brianteeman can you confirm so that we get that in before core.js changes again? :smiley:


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

avatar brianteeman brianteeman - test_item - 18 May 2016 - Tested successfully
avatar brianteeman
brianteeman - comment - 18 May 2016

I have tested this item :white_check_mark: successfully on 83339ce


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

avatar brianteeman brianteeman - change - 18 May 2016
Status Pending Ready to Commit
avatar brianteeman
brianteeman - comment - 18 May 2016

RTC


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

avatar joomla-cms-bot joomla-cms-bot - change - 18 May 2016
Labels Added: ?
avatar rdeutz rdeutz - change - 18 May 2016
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
avatar rdeutz rdeutz - close - 18 May 2016
avatar rdeutz rdeutz - merge - 18 May 2016
avatar joomla-cms-bot joomla-cms-bot - close - 18 May 2016
avatar joomla-cms-bot joomla-cms-bot - change - 18 May 2016
Labels Removed: ?

Add a Comment

Login with GitHub to post a comment