No Code Attached Yet Information Required bug
avatar PhocaCz
PhocaCz
8 Sep 2021

Hi,

is there a final solution for this problem:

img

line 157

// We have to move the modal, otherwise we get problems with the backdrop
			// TODO: There should be a better workaround than this
			Factory::getDocument()->addScriptDeclaration(
				<<<JS
document.addEventListener('DOMContentLoaded', function() {
  var modal =document.getElementById('{$options['selector']}');
  document.body.appendChild(modal);
  if (Joomla && Joomla.Bootstrap && Joomla.Bootstrap.Methods && Joomla.Bootstrap.Methods.Modal) {
    Joomla.Bootstrap.Methods.Initialise.Modal(modal);
  }
});
JS
			);

It is OK for core methods, but it does not solve the general using of HTMLHelper::_('bootstrap.renderModal', ...

Using HTMLHelper::_('bootstrap.renderModal', ... generally produces the problem with backdrop and z-index.

Changing method renderModal in abstract class Bootstrap solves the problem
(libraries/src/HTML/Helpers/Bootstrap.php 774)

public static function renderModal($selector = 'modal', $options = [], $body = '') :string
	{
		// Only load once
		if (!empty(static::$loaded[__METHOD__][$selector]))
		{
			return '';
		}

		// Initialise with the Joomla specifics
		$options['isJoomla'] = true;

		// Include Basic Bootstrap component
		HTMLHelper::_('bootstrap.modal', '#' . preg_replace('/^[\.#]/', '', $selector), $options);

		$layoutData = [
			'selector' => $selector,
			'params'   => $options,
			'body'     => $body,
		];

		static::$loaded[__METHOD__][$selector] = true;

// START EDITED SECION ----
		// We have to move the modal, otherwise we get problems with the backdrop
			// TODO: There should be a better workaround than this
			Factory::getDocument()->addScriptDeclaration(
				<<<JS
document.addEventListener('DOMContentLoaded', function() {
  var modal =document.getElementById('{$selector}');
  document.body.appendChild(modal);
  if (Joomla && Joomla.Bootstrap && Joomla.Bootstrap.Methods && Joomla.Bootstrap.Methods.Modal) {
    Joomla.Bootstrap.Methods.Initialise.Modal(modal);
  }
});
JS
			);
// END EDITED SECION ----

		return LayoutHelper::render('libraries.html.bootstrap.modal.main', $layoutData);
	}

But the question is, is this really a final well implemented solution?

Thank you, Jan

avatar PhocaCz PhocaCz - open - 8 Sep 2021
avatar joomla-cms-bot joomla-cms-bot - change - 8 Sep 2021
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 8 Sep 2021
avatar PhocaCz PhocaCz - change - 8 Sep 2021
Title
Final solution for class PopupButton method renderButton line 157
[4.0] Final solution for class PopupButton method renderButton line 157
avatar PhocaCz PhocaCz - edited - 8 Sep 2021
avatar PhocaCz PhocaCz - change - 8 Sep 2021
The description was changed
avatar PhocaCz PhocaCz - edited - 8 Sep 2021
avatar PhocaCz PhocaCz - change - 8 Sep 2021
Title
[4.0] Final solution for class PopupButton method renderButton line 157
[4.0] Final solution for Modals - e.g. class PopupButton method renderButton line 157
avatar PhocaCz PhocaCz - edited - 8 Sep 2021
avatar dgrammatiko
dgrammatiko - comment - 8 Sep 2021

There was a solution but it was rejected: #32062

BTW your proposal breaks at least the fields: user, media and modals

avatar PhocaCz
PhocaCz - comment - 8 Sep 2021

Hi, I didn't add any proposal in the post. I asked for the final solution. It looks like a temporary solution is being used at some point (maybe it's at multiple locations). Such a temporary solution, when used in the Bootstrap method, works, but not systematically and only for a specific function (if it breaks some other part, so much the worse).

So if the #32062 was rejected, what is then the right way for using HTMLHelper::_('bootstrap.renderModal', ...

Of course, I can divide the output in my extension easily (separate the html and paste this part to the bottom of site) and it works OK, but I would like to know the proper way of using HTMLHelper::_('bootstrap.renderModal', ...

Thank you.

avatar dgrammatiko
dgrammatiko - comment - 8 Sep 2021

No what you’re proposing is fundamentally wrong. The htmlhelper fn is echoing the string wherever you are calling it and by moving the dom element with js you are producing a really unpredictable behavior both for js and also for css.

The other pr was closed because it was a side effect of my work on the bootstrap 5 and at that time we didn’t had the time to spare for this. Also it’s the right way, you don’t need js for this

edit: rejected by me…

avatar PhocaCz
PhocaCz - comment - 9 Sep 2021

No what you’re proposing is fundamentally wrong. The htmlhelper fn is echoing the string wherever you are calling it and by moving the dom element with js you are producing a really unpredictable behavior both for js and also for css.

Can you read what I'm writing? I'm not suggesting/proposing anything, this is code that is part of Joomla 4 core and we agree that it is not correct, that's why I'm asking what is the correct solution?

libraries/src/Toolbar/Button/PopupButton.php ... line 158 ... THIS IS NOT MY PROPOSAL, IT IS A JOOMLA 4 CORE CODE solving some part of the system but leaving other parts unsolved.

avatar dgrammatiko
dgrammatiko - comment - 9 Sep 2021

Changing method renderModal in abstract class Bootstrap solves the problem

@PhocaCz actually you are proposing the hack that is applied only in one case to be the norm for all the modals. As I said the solution needs to be in the HTML, you don't need JS for this, just enqueue the modal at the end of the body from the PHP side.

avatar PhocaCz
PhocaCz - comment - 9 Sep 2021

I don't propose it, I only added it as example, that applying the same hack just work, nothing more.

Can you read, what I wrote?

Of course, I can divide the output in my extension easily (separate the html and paste this part to the bottom of site) and it works OK, but I would like to know the proper way of using HTMLHelper::_('bootstrap.renderModal', ...

#35506 (comment)

So I have no problem to "just enqueue the modal at the end of the body from the PHP side". The only problem is, it makes the code very unclear and unreadable in comparing to Joomla 3 usage.

avatar dgrammatiko
dgrammatiko - comment - 9 Sep 2021

I don't propose it, I only added it as example, that applying the same hack just work, nothing more.

It might solve your own button problem but as I wrote above it will break at least 3 core fields and also it's unexpected if you echo something in the middle of the page some hacky JS moves it to the body end (it could break any styling that depends on the cascading).

Let me reiterate the problem is that Joomla doesn't have a way to enqueue html chunks to be placed at the bottom of the body.

The only problem is, it makes the code very unclear and unreadable in comparing to Joomla 3 usage.

What's unclear? Instead of having a direct echo that will place the chunk of HTML wherever you called it, now you have to enqueue the HTML chunk to be placed/echoed at the body end. Also what's unreadable? You're passing the result of a function to another function, which Joomla is doing all over...

avatar PhocaCz
PhocaCz - comment - 9 Sep 2021

Look, I'm not here to explain what is readable and unreadable, clear or unclear. I am simply rewriting components from Joomla 3 format to Joomla 4 format and I encounter that the standard functions stop working and I have to look for complicated, complex and unreadable solutions.

The question in this topic is clear. There is some part in Joomla 4 code which says:

// TODO: There should be a better workaround than this

and I am just asking, maybe the author of this comment, if there is better workaround for this or there is a plan to do it?

a) IF NO: I'll stick with the current hard-to-read and unclear solution
b) IF YES: Great to hear it, I will use the new better workaround

a) or b) is good enough for me.

avatar dgrammatiko
dgrammatiko - comment - 9 Sep 2021

and I encounter that the standard functions stop working and I have to look for complicated

What's not working? How did you create the button, using the API or your own constructor? If I'm reading the existing code correctly, even if it's a monkey patch, should work as long as you're using the API.

avatar PhocaCz
PhocaCz - comment - 9 Sep 2021

Joomla 3 version:

$html = array();
$bar  = JToolbar::getInstance('toolbar');
$html[] = '<a href="#SomeId" role="button" class="btn btn-small" data-toggle="modal" title="Preview"><span class="icon-ph-preview"></span> Preview</a>';

$html[] = JHtml::_(
    'bootstrap.renderModal',
    'SomeId',
    array(

        'url'    => $link,
        'title'  => 'Preview',
        'width'  => '700px',
        'height' => '400px',
        'modalWidth' => '80',
        'bodyHeight' => '70',
        'footer' => '<button type="button" class="btn" data-dismiss="modal" aria-hidden="true">Close</button>'
    )
);

$dhtml = implode("\n", $html);
$bar->appendButton('Custom', $dhtml);

(the code is simplified, there are different buttons with different custom solutions, like onclicks with different JS code, etc. even not only menu buttons so the question is not only about using menu buttons but using JHtml::_('bootstrap.renderModal', ... directly)

Joomla 4:
Changing parts like joomla-toolbar-button, data-bs-toogle, data-bs-target, href to link, etc. etc.

But JHtml::_('bootstrap.renderModal', does not call the "monkey patch", so you need to divide the HTML output here. This is even not a problem but I just ask if there is some proper way to do it with Joomla current methods.

avatar dgrammatiko
dgrammatiko - comment - 9 Sep 2021

@PhocaCz so you're doing it your way. The easiest solution is to use LayoutHelper::render('toolbar.popup', $settings); followed by the

HTMLHelper::_(
    'bootstrap.renderModal',
    'SomeId',
    array(

        'url'    => $link,
        'title'  => 'Preview',
        'width'  => '700px',
        'height' => '400px',
        'modalWidth' => '80',
        'bodyHeight' => '70',
        'footer' => '<button type="button" class="btn" data-dismiss="modal" aria-hidden="true">Close</button>'
    )
);

That way you get the money patch for free and you're using the API

avatar PhocaCz
PhocaCz - comment - 9 Sep 2021

LayoutHelper::render('toolbar.popup', $settings);

Unfortunately, as written above, I am not searching a solution for toolbars only but generally for HTMLHelper::_('bootstrap.renderModal', ... which can be used e.g. inside the list row, etc.

Another question is whether to rely on the "monkey patch", or rather separate the HTML directly (even knowing that it will be less legible)

avatar dgrammatiko
dgrammatiko - comment - 9 Sep 2021

Unfortunately, as written above, I am not searching a solution for toolbars only but generally for HTMLHelper::_('bootstrap.renderModal', ... which can be used e.g. inside the list row, etc.

Ok, for the toolbars I gave you a viable solution that is doable right now, no patch or whatever needed in the core. For anything that belongs to your component's output you can replicate the inline JS for any parts that having conflicting z-indexes.

The real solution as I said before is an API that allows enqeuing html chunks to be appended at the document's body end.

avatar Hackwar Hackwar - change - 22 Feb 2023
Labels Added: bug
avatar Hackwar Hackwar - labeled - 22 Feb 2023
avatar brianteeman
brianteeman - comment - 23 Aug 2024

Is there anything to do here now?

avatar Quy Quy - change - 23 Aug 2024
Labels Added: Information Required
avatar Quy Quy - labeled - 23 Aug 2024

Add a Comment

Login with GitHub to post a comment