LGTM

I left couple of comments on insertAdjacentHTML but also before this goes to production it needs the dialog polyfill, you can copy over the code from https://github.com/dgrammatiko/joomla-modal/blob/main/src/index.js and add the package in the package.json (https://github.com/GoogleChrome/dialog-polyfill)

also before this goes to production it needs the dialog polyfill,

At time when it will happen, we will be no need polifill ?
Well, yeah sure, can be added, but as last thing here, not really important for now.

Well, yeah sure, can be added, but as last thing here, not really important for now

Sure, so apart the innerHML part everything else are spot on on what we had discussed. Maybe the anchor shouldn't be included, for a11y reasons, ie: the dialog should be opened from a button but that's bikeshedding that someone else should do.

I will try to play a bit with it tomorrow but I think it's already in very good shape

@Fedik one little change: could you rename this to JoomlaDialog and all the instances of popup to dialog? The reason is that later this year the platform will have (already on chrome) the popover so this should be easily distinguishable

I see, makes sense, yes I will rename

@laoneo could we have a decision for 4.4 here? This PR will unlock the fixes needed all around the broken repeatables, modal fields, etc
There's an actual implementation for the fields User and Media here: #40152

@dgrammatiko what for a decisions you expect? If this can go into 4.4? As @Fedik already mentioned in the pr description, this should finally be merged into 5.0 as 4.4 will not get any new features.

So 4.3 was the last minor that had new features? Well, ?

What is the problem to ship it with 5.0? They will be released at the same time.

What is the problem to ship it with 5.0? They will be released at the same time.

I guess some dev will be complaining, like the did with the WAM, that they need 2 different code snippets in order to support both v4 and v5...

As long as the old way still works, they should then use that. And yes, when they want the new dialog/popup/modal, then they need to have different code which is dependent on the Joomla version. I don't see this as a big issue.

As long as the old way still works, they should then use that.

The old way (bootstrap) works but devs SHOULD NOT use them for fields, editor buttons, etc that are already broken right now because this approach is doing SSR (Server Side Rendering) of the modals which are incompatible with repeatable fields. Same goes for the category fields for different reason. My point is that the script should be introduced in the 4.x so the transition would be easier...

I think 5.x+ is fine, maybe we will get some better ideas untill then.
And I would like to finish #40082 before this, and try different approaches for (cross)editor buttons.

And I would like to finish #40082 before this, and try different approaches for (cross)editor buttons.

I was thinking doing the first implementation with the buttons based on this PR and the #40082 but it needs also extending the Joomla.editors to have something like

Joomla.editors = {
  instances: {},
  buttonsCallbacks: {
    ['plg_editors-buttons_readmore']: () => { /* the code for the callback */ },
  },
}

I thinking about something generic, that can handle basic stuf without extra coding, and allow to be extended when needed.

I thinking about something generic, that can handle basic stuf without extra coding, and allow to be extended when needed.

My idea for the editor buttons was:

• create a new plugin group, ie editor-buttons and for J4/5 check both, with J6 only editor-buttons would exist
• The plugin instead of returning a php object (CMSObject) would just register the buttons in the Joomla.editors as a JS object { name, icon, etc }
• The plugin would register the onClick function on the Joomla.editors
• The plugin would also return true on the PHP side if the plugin is enabled on the particular instance of the editor
• The editor should render (client side) the buttons and bind either the modal or the onClick

This way on every repeatable instance the buttons will work as expected (the editor init function has the responsibility to handle those, ie: tiny will render them inside a toolbar but others could/would render them bellow the editing area)

Didn't look to deep into the code, but is it possible to adapt the output of a modal with a template override as you can do now with the BS ones?

Didn't look to deep into the code, but is it possible to adapt the output of a modal with a template override as you can do now with the BS ones?

It does not depend from server layout anymore. Pure "client side", almost.
Here you still can customise popup template, but in 99.99% you will be no need it.

is it possible to adapt the output of a modal with a template override as you can do now with the BS ones?

You could use a <template> element with some HTML and have the modal initiator script pick the parts (Header, body, footer) and use them. The problem with all the Fields but also with other parts of the CMS is that you end up with repeated HTML fragments. There was once a PR that could solve this with a deferred renderer #37330

It does not depend from server layout anymore.

This does mean, it doesn't follow our current way of doing, that output is generated in a specific layout file. I'm not sure if all people will be happy with this. Just want to point that out. I'm not against the new approach, but it should then be documented somehow.

I'm not sure if all people will be happy with this. Just want to point that out.

They can still use BS Modal this PR is NOT replacing the BS Modal script, the latter is still available.
For highly interactive elements (Fields, repeatable, etc) the pre-rendered html just doesn't scale/work so it needs to be abandoned and switch to client side rendering (btw the media manager has also 0 lines of php layouts and there are other instances, so the assumption that Joomla\s current way is exclusive SSR is a lie). On the other hand the project could keep everything broken because people think that it's a heresy to have client side rendered elements...

I guess it is better to rebase this pr to the 5.0-dev branch, as it will get more attention for new features. Thanks for understanding.

@Fedik can you rewrite/change a joomla action (maybe com_content modal select field) so I can have a better feeling how this works with the joomla code base?

I expect you only have to change the layout of the field right?

@HLeithner try this #40152

@HLeithner try this #40152

this looks really complex for using a simple popup/modal?!

this looks really complex for using a simple popup/modal?!

@HLeithner not really, these 2 fields have a dependency on the BS Modal as of 4.3.0. The changes to convert them to the new one are not complex but maybe this is not the easiest example. The easiest example would be some toolbar popup button, where the code would be significantly less or some of the move/copy buttons (eg banners, articles, etc). I'm quite busy atm to code such an example, but for what is worth, this approach of the modals has my support (it's an iteration/improvement of my exploratory PR and according to the specs that were set in the discussion). In sort it's a step forward, it eliminates a number of existing XSS and it's a better fit for the project as it's tailored to the actual needs...

If you remamber SqueezeBox from < joomla 2.5, that will be similar.

I expect you only have to change the layout of the field right?

For basic stuff yes.
But, for make it "good" there also will be need some JS changes.

That one #40202 based on current pr, it uses new popup for all XTD buttons (except media).

This is shared "modal" action, for XTD buttons, that replaces layouts/joomla/editors/buttons/modal.php which rendered for every modal button.

rewrite/change a joomla action (maybe com_content modal select field)

com_content "modal select" field is pretty complex field,
that is heavily coupled with horror inside media/system/js/fields/modal-fields.js
Would be a bad example ?

This will probably require multiple steps for updated that.

This will probably require multiple steps for updated that.

It can be in one step but the PHP side of the code introduces B/C breaks: #39351

But, yeah, that's the worst example for showcasing the modals ?

@HLeithner Here is simpliest example that I found :)
5.0-dev...Fedik:joomla-cms:popup-guidedtour-viewall

Replacing modal for "Show all tours".

@HLeithner there I tried more simple example #40359 :)

thanks, I tried this pr and on the second click it was broken (guidedtour branch) ;-) you can't open a popup again.
Also in the guided tour plugin you removed the code that handles the tab-path out of modal

you can't open a popup again

Yes there was a bug in Dialog script, I have fixed that, in one of last commits here. You probably have to re-run 'npm' on new changes.

Also in the guided tour plugin you removed the code that handles the tab-path out of modal

We do not need it anymore with Dialog, just try, the "tab" should work as usual.

@HLeithner there #40462 a prototype of what I have in mind for Modal_Article field (similar will be for Category, Menu and other Modal_x)

every time I see this pr I wonder why we have an RFC about Credit Cards

every time I see this pr I wonder why we have an RFC about Credit Cards

pretty sure we need credit card support

Sorry to be late to the party. Is there a reason why we don't use the new dialog html element? https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog

From what I can tell this works as per the instructions

Is there a reason why we don't use the new dialog html element?

What do you suggest?

One question, shouldn't this be part of our Joomla Super Global Object/Namespace instead of the JoomlaDialog -> joomla.dialog?

One question, shouldn't this be part of our Joomla Super Global Object/Namespace instead of the JoomlaDialog -> joomla.dialog?

Actually the custom element should be exported (without being registered in the registry) and then whatever script needs the modal should import the script (using the importmaps) and have some code (pseudo) like:

import JoomlaDialog from 'Joomla-dialog';

if (!customElements.get('joomla-dialog')) {
  customElements.define('joomla-dialog', JoomlaDialog);
}

// code specific to the script...

JoomlaDialog -> joomla.dialog

JoomlaDialog is a class that represent a custom element in DOM, there no point to place it under Joomla.
It may live without Joomla very well (under some conditions).

It kind of <img>:

const img = new Image;
img.src = 'blabla.png';

But for <joomla-dialog>

Actually the custom element should be exported

I thought about it, but would keep it as 1 file, for now.
We can split it at any time, if there will be a real demand.

@Fedik re checking the code what I meant was:

• keep it as one file
• replace window.JoomlaDialog = JoomlaDialog; with export default JoomlaDialog;
• add the script also as an importmap

So any other script that needs to construct a dialog will do:

import JoomlaDialog from 'Joomla-dialog';

const myDialog = new JoomlaDialog(someConfig);

// the rest...

hmhm, yeah that can work.
I just not sure if doing a registration of anything in DOM while doing import is a good idea.

I will do some tests, and probably will update the PR

Okay, I made it as a module:

import JoomlaDialog from 'joomla.dialog';

const dialog = new JoomlaDialog();
dialog.textHeader = 'The header';
dialog.popupContent = '<strong>blabla very strong text</strong>';
dialog.show();

Any other sugestions/wishes before I update Manual PR?

2 things would be great.

1. A css for frontend independen from the template because it's use less now in frontend
2. footerText adding text or html to the left side of the footer (like the legend for codemirror)
3. Maybe a backdrop?

I tested it and it works as expected.

hmm, I thought more on a css file I can use in my own frontend template as base and not only cassiopia, what do you thing?

By default it use a browser default styling for dialog.
I have fixed iframe sizing, now it pretty usable without extra css.

Backdrop also there, it is browser default.

About footerText. I not very want to overload with features, and keep it simple.
However it still possible to customize:

const dialog = new JoomlaDialog();
dialog.textHeader = 'The header';
dialog.popupContent = '<strong>blabla very strong text</strong>';
// Customise footer
dialog.getFooter().insertAdjacentElement('afterbegin', (() => {
  const el = document.createElement('div');
  el.innerText = 'Footer text';
  return el;
})());
dialog.getFooter().classList.remove('empty');
dialog.show();

thanks, I think it's a starting point, even if I would add a css which is (or could be) part of the webasset with a nicer styling

Does this issue have something to do with this topic here?

#42291

Hey, since JoomlaDialog is now a module script I think it would be useful to have a little section in the manual that shows how this class can be used in "classic" non-module scripts, as the import function can only be called from another module script. The current manual gives the impression you can use the JoomlaDialog class in any script.

For example static:

<script type="module">
    import JoomlaDialog from "joomla.dialog";
    window.JoomlaDialog = JoomlaDialog;    
</script>

Or dynamic:

async function run() {
  const JoomlaDialog = await import("joomla.dialog");
  dialog = new JoomlaDialog();
}

I think it would be useful to have a little section in the manual

@Frank-MandarinFish As far as i know everybody can contribute to the manual