i follow at this step...

but when i click on breakcum... this popup said wrong at tile name.

and sometime when i'm click other position, it auto turn back...


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

When choose Module Assignment, when click module Main Menu, it turn back main : Edit Menu Item.

When choose module "Similar Items (Public in )" in Module Assignment, popup don't show right module.

I have the same trouble that trangredweb, When I choose module like "breadcrumbs" in module assigment, popup don't show right module, and when I close this modal and choose another module go back (closing modal and select details tab).

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

I have tested this patch, fasing same issue mentioned by trangredweb & jcata .

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

The first error here.
and hey @trangredweb xD

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

@LLNet @mrunalpittalia @jcata @trangredweb I fixed the rendering the wrong module problem, can you test again?

Mind you that the styling might be off here because, this PR is part of all the others with the modals, but this one NOW will use the standard css, which of course will be too narrow. you can copy/paste the template.css from here https://github.com/joomla/joomla-cms/pull/4561/files

@test successfully

Step1 :- Access Comoponent menus like in url (?option=com_menus&view=menus).
Step2 :- select Linked Modules->modules(DropDown)->main menu(public in position 7) before press ctrl+shift+m.
Step3 :- if u press above shortcut then layout come as bootstrape or mobile layout
Step4 :- Then after select Step2.....

So test Successful and i cant find any bugs related to bootstreps...

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

@test successfully, BUT, Step1 :- Access Comoponent menus like in url (?option=com_menus&view=menus).

Notice: Undefined property: MenusViewMenus::$id in /home/pwork02/9_Testing/j3/administrator/components/com_menus/views/menus/tmpl/default.php on line 32

Generates this Javascript function "function jSelectPosition_(name) {", note that the ID is missing

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

@anibalsanchez That code was already there. I’ll see what I can do

@test Works successfully with new pull, tested successfully.

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

@test sucessfully works with bootstrap without any issue..

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

@anibalsanchez no notice anymore!

Moving to RTC as we have more than 2 successful tests

Note: counted successful test after new patch from dgt41

@Bakual Did I just kill travis?

Relaunched it. Let's see.

Travis kills itself nowadays
There is an issue with Travis authenticating against GitHub for some reason. Looks like some rate limiting to me.
It happens now because part of Joomla is now powered using composer.

tested: I get
( ! ) Parse error: syntax error, unexpected end of file in root/administrator/components/com_menus/views/menus/tmpl/default.php on line 205

as a <?php endif; ?> is missing after </table>
also in this file, please respect indentation.
also
Notice: Undefined variable: canEdit in /root/administrator/components/com_menus/views/menus/tmpl/default.php on line 39

Also, I can't access to the Note field at bottom right of the modal, except if I enlarge drastically the window height

Also
( ! ) Notice: Undefined variable: item in ROOT/administrator/components/com_menus/views/item/tmpl/edit_modules.php on line 17

@infograf768 Now everything should be fine

We still have the modal sizing error:

and another Notice:
Notice: Trying to get property of non-object in root/administrator/components/com_menus/views/menus/tmpl/default.php on line 41

(that one can be solved by changing back in default.php to
foreach ($this->modules[$item->menutype] as &$module) :

@infograf768 All the iframes heights should be 600px, is that way too big? maybe some browser caching…
I reverted that foreach line to get rid of the notice

Hathor views:

As strange as it may seems, applying this patch (with com_patchtester to J! 3.3.6) has side effects on the administrator login page: the background attributes for the .view_login CSS class are lost!

@smanzi But we don’t touch at all the login view and the additional css only affect the modals. Maybe a glitch in com_patchtester?
Here is the before

and the after

In my localhost. Which seems identical except that the colors for some reason (unknown to me) are altered

Yeah... strange... in my template.css, after applying this PR I have (at line 6991):

.view-login {
    padding-top: 0;
}

@smanzi I confirm that this code is added somehow in the template.css (strange)
If you just save again the style in isis the colors are restored…
I guess is the com_patchtester

@dgt41 Sorry I must be really thick today, but... what do you mean by "save again the style in isis"?

Also:
Giving the iframe an height of 500px IMHO in many situation determines a loss of available real estate and the need to use the scroll bar which will not be needed if all the available screen real estate was used.

I'm now working on a proof of concept that will use all the available real estate (at this time I'm only patching the generated HTML "live" with firebug): the concept is not giving an "height" to the iframe and instead giving it a style="height:95%". The same must be done to every containing div, and a height:5% must be given to the modal-header div.

It works, somehow better, but there are issues if that 5% allocated to the header becomes not enough for its actual height.

I also find the <div id="module##Modal-container"> to be useless.
Instead, by putting both the modal header and modal container inside a further containg div, would be possible to center the modal with a "margin: 0 auto", which I find more elegant that using relative positioning with the "left: 50%" attribute...

Stay tuned! :-)

@smanzi On the last one: the html code is generated from javascript so what you propose is to alter the bootstrap modal code and I am not so sure if people will like that.
As for save:
go to Extensions->templates->isis default and just press the save button

as far as regards the template.css "glitch", I tried what you said, even forced reloading the css, but I still get a white login page... but let's forget about it if it is just a com_patchtester issue (to be investigated, in its context, anyway... who is maintaining it?)

Generated HTML: I get what you mean, and no, it wouldn't be a good Idea. Anyway that 500px height is something I really dislike: what about adding a small JQuery script acting on the DOM and "fixing things up"?

@smanzi If you have some code don’t hesitate to bring it either here or in another PR. Also there are these PR’s #4645 #4575 #4563 dealing with the same css code, so if we can do something js wise to bring some sanity will be excellent! Let me know if you have something!

Yes, I've seen also the other PRs, but I started looking at this one...

OK, If I came to something useful I'll let you know, either here or with a PM: I don't want to mix things up too much with new PRs...

Thanks!

is this the repo for com_patchtester: https://github.com/ianmacl/patchtester ?

is this the repo for com_patchtester: https://github.com/ianmacl/patchtester ?

No, that's an outdated one.
Use http://joom.la/patchtester or https://github.com/joomla-extensions/patchtester

Thanks, Thomas!

@smanzi If it helps there is some js code you can look here: http://stackoverflow.com/questions/10169432/how-can-i-change-the-default-width-of-a-twitter-bootstrap-modal-box

@dgt41

Probably something lin the lines of this (taken from http://stackoverflow.com/questions/11873312/jquery-adjusting-iframe-height):

jQuery(document).ready(function() {
    var height = $(window).height();
    $('iframe').css('height', height * 0.9 | 0);
});

We should anyway, do something more "refined" and take into account the height of the .modal-header div as well...

... and maybe assign an ID to the iframe to target it better...

@smanzi I am afraid we don’t have the option to set ID in the iframe https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/html/bootstrap.php#L318-L319
But this can change

Folks
#4661 (comment)

.view login has changed in 3.4 and this has been ported to staging after the merge.

.view-login {
    padding-top: 0;
}

is correct.
see where it is now:
https://github.com/joomla/joomla-cms/blob/staging/administrator/templates/isis/login.php#L59

@infograf768 Thanks! I closed the joomla-extensions/patchtester#53 issue.

P.S.: why the "padding-top: 0;" attribute wasn't put in the inline style??

@dgt41
I'm afraid I'm going to throw in the towel:

My idea was to add into a "jQuery(document).ready(function() {});" something like:

$script[] = "jQuery('.modal iframe').css('height','" . mycomputedheight . "px !important')";

Unhappily I'm even unable to understand where and which JHtmlBootstrap::renderModal call in which file is used in this context as there are several, all similar between themselves, throughout the code.

I'm afraid I'm not ready yet for this maze... sorry... :-(

@dgt41
Well... actually I didn't throw the towel in!

In administrator/components/com_menus/views/item/tmpl/edit_modules.php try adding the following after line 22:

    $script[] = '   jQuery("#module'. $module->id .'Modal").on("show", function () {';
    $script[] = '       jQuery(".modal-body iframe").height(jQuery(window).height() * 0.8);';
    $script[] = '       jQuery(window).resize(function () {';
    $script[] = '           jQuery(".modal-body iframe").height(jQuery(window).height() * 0.8);';
    $script[] = '       });';

That 0.8 is only a first (rough) estimate of what the height should be, but it seems to me that the damn thing is working... It resizes too when resizing the browser window.

What do you think?

Unrelated with the "height" issue:

It would be better not having the "data-dismiss" button on the modal and solely rely on the "Save" and "Cancel" buttons as using the data-dismiss "X" for the modal will leave the associated module locked.

This, anyway requires a modification of JHtmlBootstrap::renderModal() with the addition of a parameter to select if one wants to have the data-dismiss button (default=true).

Do you think it would be worth if I make a PR for that?

... or it may be hidden through Javascript ...

@smanzi Sorry Sergio for the late response, I was trying to push some code

@dgt41
No prob, Dimitri!

Yes, I understand that it is difficult to make styling modifications and have everybody happy, but... just give a try to my code: I honestly and respectfully think it is better than the fixed 500px height... What I would like to do is to give the iframe an height so that the modal will be centered in the window (same margin below as above) but at this time I have difficulties figuring out what the top margin is through Javascript (that Boostrap modal is a real bitch!).

The same approach I used for the height can be used for the modal width, I think...

OK, I will look into modifying JHtmlBootstrap::renderModal(), but first I have to figure out if that button is not silently triggered when using the "Close" or "Cancel" buttons: if this is the case the button must stay there and eventually hidden through Javascript.

@smanzi The button is coupled to this: $(‘#myModal').modal(‘hide’) and the close button (on the form not the x) is also calling this code
See also the docs http://getbootstrap.com/2.3.2/javascript.html#modals

@dgt41 got it, thanks! This being the case, I think modifying JHtmlBootstrap::renderModal() is the right thing to do. Will do! :-)

@dgt41 I've modified JHtmlBootstrap::renderModal() adding a boolean "closebtn" parameter that when set to false will not display the close (x) button. This is working and I can make a PR for that, but also clicking outside of the modal have the same effect (closing the modal) and the same side-effect (module item locked). I would like to correct this too, either through the same parameter or through another one, but I really can't get where this is handled... :-/ Any hint?

@smanzi In which modal you get that? We can fix it in that view (will be easier)
You can Skype me @ velocity-gr

@dgt41 Sorry, going out now, but you can add me to your skype @ sergiomanzi
I will be on-line tomorrow afternoon.

I'm getting this in the modal for editing modules, but I suspect this is a global behavior of the bootstrap modal... Just try clicking outside of the modal, in the darkened space...

@dgt41 See: smz@8eb2da5 for "tentative" code for a PR for JHtmlBoostrap.

I'm saying "tentative" because the usage of a static backdrop (no close on click) doesn't seems to work when applied to this PR. I have doubt about the generated JS in modal() (untouched by me...)

Info about the static backdrop can be found in:
http://getbootstrap.com/2.3.2/javascript.html#modals
http://stackoverflow.com/questions/9894339/disallow-twitter-bootstrap-modal-window-from-closing#answer-11443560

@smanzi I think will be easier to do something like this

        jQuery('.modal').on('hidden', function () {
            Joomla.submitform('module.cancel', document.getElementById('module-form'));
        });

On the modal views. Haven’t try it, but in theory should work...

@smanzi If you have spare time check my solution, it works here

@dgt41 Hi Dimitri!
I was thinking too something in your line of thought: to handle the modal .on('hidden') event so that it triggers module.cancel. This would have the added benefit of not needing to eliminate the "x" close button of the modal and allow a more intuitive navigation. All hiding action ("x", backdrop click, keyboard escape) would be nicely handled.

Anyway I tried to insert your JS but it didn't work for me. I'm anyway quite sure I've put it in the wrong place: administrator/components/com_menus/views/item/tmpl/edit_modules.php inside the jQuery(document).ready( function() { ... } ); Where did you put it?

Got it! It works! :-)

Another concern is: what does it happens when the modal is closed because of the "Save" button? Is the on('hidden') event triggered in this case too? If yes, would this mean that we have a "cancel after save"? Will this be a problem?

Is your "TODO" related to "going back to the modules tab"?

Also, I don't understand the function of the last line in administrator/components/com_menus/views/item/tmpl/edit_modules.php: what modal should this generate??

I've added you to my Skype contacts.

@dgt41 Dimitris, I have a functioning code for centering the modal:

add this to the script declaration in administrator/components/com_menus/views/item/tmpl/edit_modules.php:

$script[] = "   jQuery(document).ready(function() {";
$script[] = "       jQuery('.modal').on('show', function () {";
$script[] = "           var contentHeight = jQuery(window).height() * 0.9;"; // 0.9 because we have CSS "div.modal {top: 5%}"
$script[] = "           var headerHeight = jQuery(this).find('.modal-header').outerHeight();";
$script[] = "           jQuery(this).css({'height': function () { return (contentHeight)}});";
$script[] = "           jQuery(this).find('iframe').css({'height': function () { return (contentHeight - headerHeight - 60)}});"; 
// The -60 in the above formula is necessary because of the stupid CSS ".modal-body {padding: 1%;}"
// It is a rough estimate that seems to work quite well for most window sizes
$script[] = "       });";
$script[] = "   });";

It is a bit more complicated than expected because we must set the iframe height. This is because we cannot set it at 100% as this is possible for iframes only if their parent <div> is also set at 100%, which is not the case.

So far I've been unable to write a corresponding jQuery(window).on(resize) function: I'm unable to target the correct elements.... :-(

IMHO, there are some stupid things in Isis: e.g. .modal-body {padding: 1%;} What the @!@#!%!!!

Also the vertical scroll bar for the iframe is always there, even when unneeded: I can't figure out why. Some padding somewhere??

Cheers! Sergio

@dgt41 alternatively, this code just center the modal vertically without modifying the declared iframe height. You loose the "slide-in" effect, anyway...

$script[] = "   jQuery(document).ready(function() {";
$script[] = "       jQuery('.modal').on('show', function () {";
$script[] = "           jQuery(this).css({'top': function () { return ((jQuery(window).height() - jQuery(this).outerHeight()) / 2)}});";
$script[] = "       });";
$script[] = "   });";

@dgt41 Dimitris, you have bad formatting in /libraries/cms/form/field/moduleposition.php at lines 160, 161 & 162: too many tabs??

@smanzi Thanks Sergio. The problem with this (as my other older PR’s) is that is made as a branch of my fork, that means if I touch it will be ruined. I guess whoever going to commit this, will have to manually erase these lines. I’m sorry, about that, it took me long time to realize that I was doing it wrong

@dgt41: wait a minute...
I do the same: fork -> create local copy -> create branch -> make mods -> commit -> sync to GitHub -> create PR.
Now, if you make further mods on your branch and commit them (and sync with your remote repo on GitHub), the mods will be automatically pushed to the already existing PR

I don't understand what your problem is...

BTW, those lines are not to be erased: just pull the code to the left...

I am lazy and I use gui Tower, which obviously for some time I misused it...

I strongly recommend that #3918 gets merged before this one!

@dgt41 I don't know if it is a mistake I'm doing, but it seems to me that applying this now (against current staging) does screw-up a lot of things...

I see things that should be unrelated (even backend templates css) and that are actually modified. Also many of your PRs are affected...

As it is now it seems to be unusable...

Yes there are some conflicts. I guess I have to wait for your PR on bootstrap modal get merged before I bring this to current staging

catch 22! I was willing to use this PR to test my modal (which actually is becoming more a Roberto's thing that mine, with his latest PR against me - VERY nice PR, btw!!)

No problem: I will test it with com_finder that is using bootstrap modal as well...

@roland-d can you tell me where, how can I find the conflicts here?
Thanks

@dgt41 Travis says:
FILE: ...mla-cms/administrator/components/com_menus/views/menus/tmpl/default.php

FOUND 1 ERROR(S) AFFECTING 1 LINE(S)

168 | ERROR | Concat operator must be preceeded by one space

This is updated so some tests to make this happen will be really appreciated!

Testing "Menu Item Type" Select button in menu item edition:

Object[]
ReferenceError: Joomla is not defined
})(jQuery, Joomla);

@anibalsanchez can’t reproduce it here. Can you clear browser’s cache and retry?

when i check this patch 4661, i have follow error. menu manager -> search field: when i write a menu name without result. then get i this error

@knoby2015 without this patch the search was working ok?

@knoby2015 @anibalsanchez I think now everything is fine, can you retest?

@anibalsanchez tx it´s running well

@test ok in a fresh installation

@losedk Can I ask for a test here, so we move this to rtc?

@test all good!

The last chages are successful tested. Moving here to RTC. Thanks!

@zero-24 can you RTC this?

@dgt41 it is RTC on issues.joomla.org: http://issues.joomla.org/tracker/joomla-cms/4661 ;)

@zero-24 Oops my bad!

I had another problem with the code changes of this pull request here.

If you try to add a new menu item of type Single article for instance, then change to the tab Module Assignment to open a module for editing, close it again and change to the tab Details back again you will miss the Select button for the field Select article.
I suspect it's because of the introduction of window.parent.jQuery('.modal').modal('hide'); at some places of the code.

@Erftralle Thank you so much for finding those bugs, everything should be fine with #6804. I will appreciate if you can test that PR