[#23062] - [4.0] Revert to jQuery in menus module selection till CE modals are used
- Fixed in Code Base
- 13 Dec 2018
- Medium
- Build: 4.0-dev
- # 23062
- Diff
- Digital-Peak:j4/fix/menu/reload
User tests: Successful: Unsuccessful:
Pull Request for Issue #22967.
Summary of Changes
Revert to jQuery till the modal in the menus module selection is created by a custom element. Till then it fixes the JS error mentioned in #22967 and reloads the page after the modal gets closed.
Testing Instructions
- Open /administrator/index.php?option=com_menus&view=menus
- Click on Modules in the "Linked modules" column
- Click on the first entry
- Wait till the modal has opened
- Click on cancel
Expected result
The page reloads after a second.
Actual result
The page does not reload.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_menus JavaScript Repository |
| Labels |
Added:
?
|
||
Will it not be broken anyway?
Just use the snippet I gave you
I'm not addung somwrhing which is not needed qhen we have the jQuery dep. anyway. We can do it right when we move to CE.
Then when that code will be vanilla it will be broken. Jquery is something used here just because the event is not parpagating to the window. Also if you don’t commit that code I guess you will have to find someone that understands how to fix that problem later on, which, in the j-world is not very easy. Anyways you asked me and I provided you the snippet, if you’re not gonna use it I guess I just wasted my time once again
@laoneo the problem here that it will reload a page, doesn't matter which modal was closed. That unexpected behavior.
@dgrammatiko I think it can be more simple
In general the bug because of incorrect use of Joomla.Modal.getCurrent().
Please hold this, for now, (not close).
I will try check more, when will get some time
That unexpected behavior.
@Fedik actually the part of the page with the modules should be Ajax driven and in that case the page reload is useless. But requires some heavy refactoring.
In general the bug because of incorrect use of
Joomla.Modal.getCurrent().
Unfortunately the event (bs.hidden in our case) is tightly coupled to the modal element, it's not a document event (and honestly specificity is good).
@Fedik actually there is no need for mutation observers here:
const modals = [].slice.call(document.querySelectorAll('.joomla-modal'));
modals.forEach((modal) => {
console.log(modal)
modal.addEventListener('hide.bs.modal', () => {
setTimeout(() => { window.location.reload(); }, 1000);
});
})
@dgrammatiko even better to use a Promise
@C-Lodder the problem is that Bootstrap's events do fire in the jQuery context: https://codepen.io/dgrammatiko/pen/bQqzGx
Other than that Joomla hardly ever does anything in the client side, almost all of the html is server side rendered thus mutation observers and promises are hardly required. Exception here the custom elements that actually ALL of them needs to be rewritten with the usage of mutation observers!
To recap here:
Replace lines 21-24 in the admin-menu script with:
const modals = [].slice.call(document.querySelectorAll('.joomla-modal'));
modals.forEach((modal) => {
console.log(modal)
modal.addEventListener('hide.bs.modal', () => {
setTimeout(() => { window.location.reload(); }, 1000);
});
})and also patch the bootstrap-init in the modal part so that events are actual javascript events and not only jQuery:
if (modal.length) {
$.each($('.joomla-modal'), function() {
var $self = $(this);
// element.id is mandatory for modals!!!
var element = $self.get(0);
// Comply with the Joomla API
// Bound element.open()
if (element) {
element.open = function () {
return $self.modal('show');
};
// Bound element.close()
element.close = function () {
return $self.modal('hide');
};
}
$self.on('show.bs.modal', function() {
element.dispatchEvent(new Event('show.bs.modal', {"bubbles":true, "cancelable":false}))
// Comply with the Joomla API
// Set the current Modal ID
Joomla.Modal.setCurrent(element);
// @TODO throw the standard Joomla event
if ($self.data('url')) {
var modalBody = $self.find('.modal-body');
var el;
modalBody.find('iframe').remove();
// Hacks because com_associations and field modals use pure javascript in the url!
if ($self.data('iframe').indexOf("document.getElementById") > 0){
var iframeTextArr = $self.data('iframe').split('+');
var idFieldArr = iframeTextArr[1].split('"');
if (!document.getElementById(idFieldArr[1])) {
el = eval(idFieldArr[0]);
} else {
el = document.getElementById(idFieldArr[1]).value;
}
var data_iframe = iframeTextArr[0] + el + iframeTextArr[2];
modalBody.prepend(data_iframe);
} else {
modalBody.prepend($self.data('iframe'));
}
}
}).on('shown.bs.modal', function() {
element.dispatchEvent(new Event('shown.bs.modal', {"bubbles":true, "cancelable":false}))
var modalHeight = $('div.modal:visible').outerHeight(true),
modalHeaderHeight = $('div.modal-header:visible').outerHeight(true),
modalBodyHeightOuter = $('div.modal-body:visible').outerHeight(true),
modalBodyHeight = $('div.modal-body:visible').height(),
modalFooterHeight = $('div.modal-footer:visible').outerHeight(true),
padding = $self.offsetTop,
maxModalHeight = ($(window).height()-(padding*2)),
modalBodyPadding = (modalBodyHeightOuter-modalBodyHeight),
maxModalBodyHeight = maxModalHeight-(modalHeaderHeight+modalFooterHeight+modalBodyPadding);
if ($self.data('url')) {
var iframeHeight = $('.iframe').height();
if (iframeHeight > maxModalBodyHeight){
$('.modal-body').css({'max-height': maxModalBodyHeight, 'overflow-y': 'auto'});
$('.iframe').css('max-height', maxModalBodyHeight-modalBodyPadding);
}
}
// @TODO throw the standard Joomla event
}).on('hide.bs.modal', function() {
element.dispatchEvent(new Event('hide.bs.modal', {"bubbles":true, "cancelable":false}))
$('.modal-body').css({'max-height': 'initial', 'overflow-y': 'initial'});
$('.modalTooltip').tooltip('dispose');
// @TODO throw the standard Joomla event
}).on('hidden.bs.modal', function() {
element.dispatchEvent(new Event('hidden.bs.modal', {"bubbles":true, "cancelable":false}))
// Comply with the Joomla API
// Remove the current Modal ID
Joomla.Modal.setCurrent('');
// @TODO throw the standard Joomla event
});
});
}There you have it
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23062.
@dgrammatiko yeah, it provide, but I do not see much sense in it,
while we use bootstrap.js we will be need jquery dependency anyway.
And if we do that hack, it will be harder to remove at later time, because all events will be namespaced to bootstrap 'xx.bs.modal'
@dgrammatiko that also can be done at any time, when this modal will be replaced (if it happen)
btw, @laoneo also have a @TODO, so all fine for now
not a critical issue
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23062.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23062.
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-12-13 18:07:55 |
| Closed_By | ⇒ | wilsonge |
Thanks guys
@laoneo please add the mutataion observer as well otherwise this will be broken once you move to custom elements