User tests: Successful: Unsuccessful:
Joomla.Modal = {
/**
* *****************************************************************
* Modals should implement
* *****************************************************************
*
* getCurrent Type Function Should return the modal element
* setCurrent Type Function Should set the modal element
* current Type {node} The modal element
*
* USAGE (assuming that exampleId is the modal id)
* To get the current modal element:
* Joomla.Modal.current; // Returns node element, eg: document.getElementById('exampleId')
* To set the current modal element:
* Joomla.Modal.setCurrent(document.getElementById('exampleId'));
*
* *************************************************************
* Joomla's UI modal uses `element.close();` to close the modal
* and `element.open();` to open the modal
* If you are using another modal make sure the same
* functionality is bound to the modal element
* @see media/legacy/bootstrap.init.js
* *************************************************************
*/
current = '',
getCurrent: function() {
return this.current;
},
setCurrent: function(element) {
this.current = element;
}
};
joomla-field-media
and joomla-field-user
jQuery('bla').modal('hide')
and jQuery('bla').modal('show')
Install a fresh j4 from this branch.
Insert some sample data.
Go to new article-> tab Publishing and open the Created By
modal.
Open your browser's console and type:
Joomla.Modal.current.close()
and then press enter!
voilà
Check also the media field (intro and full article images)
Everything should work as before, this is a change that affects (in a positive way) developers not the end users
YES. The API should be well documented!
In essence this is a replacement of the jModalClose()
that exists in J3, bound to Mootools modal and hacked to also (kinda) work with Bootstrap. So even if this is not approved we still need something to replace the jModalClose()
...
@wilsonge @mbabker @dneukirchen @Fedik let's decide what we should do here so @C-Lodder and @ciar4n have a clear path for the templates!
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_plugins com_redirect com_users JavaScript Repository Layout |
Labels |
Added:
?
?
|
Category | Administration com_plugins com_redirect com_users JavaScript Repository Layout | ⇒ | Administration com_banners com_fields com_plugins com_redirect com_users JavaScript Repository Layout |
the code will fail if modal initialized somewhere externally, I mean without element.open()
True, but we are creating an API so we can then say: use the API properly
modal element (not bootstrap) has own .open()/.close() methods, wich doing something else
Where did I saw that naming problem recently? Oh yeah in Javascript itself... Anyways the open/close can be named as jOpen/jClose (although I still like the open close, and letting people the don't want to use the Joomla's core UI components to figure out that themselves...
Now about the event driven approach: personally I don't mind but for newcomers it's harder to grasp the idea and the whole mechanics behind it. So depending what's our target audience (and I think that most people using/coding for joomla are not fluent in js) we should pick the best approach. If we can document this is=n plain english (that won't be me obviously) then we can go this way as well (and the dropping of the window.parent is a very tempting reason to follow this approach).
@dneukirchen so I updated this one to the specs we discussed in Koln.
@Fedik what do you think about this?
Title |
|
@dgrammatiko looks good, except 2 thing
Joomla.currentModal.get().close()
Joomla.Modal.setCurrent(element)
, but then you access directly to the .current
property Joomla.Modal.current.close()
. This looks inconsistent.Labels |
Removed:
?
|
you still have the code which use an old approach Joomla.currentModal.get().close()
Thanks, I missed those
but then you access directly to the .current property Joomla.Modal.current.close(). This looks inconsistent.
I thought that was what David wanted, I also like the idea of setters and getters as they are more explanatory, but I m also ok with this
I think @dneukirchen meant to use Joomla.Modal
instead of Joomla.currentModal
,
but better ask him
With getters is better, because we can have more control over it,
example can trow an Error with explanation message, when user call getCurrent()
before setCurrent()
I think @dneukirchen meant to use Joomla.Modal instead of Joomla.currentModal,
but better ask him?
Yes, but thats only about naming.
Do you want me to use the getter instead of directly calling the stored value?
@Fedik @dneukirchen I made this codepen: https://codepen.io/dgrammatiko/pen/bMzpqB
So do we agree on correcting this from Joomla.Modal.current
to Joomla.Modal.getCurrent()
in combination to the exposed properties on the modals (onOpen and onClose)?
looks good,
do we agree on correcting this from Joomla.Modal.current to Joomla.Modal.getCurrent()
yes I think use Joomla.Modal.getCurrent()
is better
Category | Administration com_plugins com_redirect com_users JavaScript Repository Layout com_banners com_fields | ⇒ | Administration com_banners com_fields com_redirect com_users JavaScript Repository Layout |
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-06-03 13:34:30 |
Closed_By | ⇒ | laoneo |
Merged to continue the JS API.
okay, looks good but still not perfect
Personally I would prefer something "event based" without these 1000 callbacks (which still can be missed at some point of time).
Something like:
In 2 words: when user click close (or the script do request for close) it will dispatch custom event
joomla:modal:doClose
against it'swindow
object (iframe window), this event will be catched by parent page, and in result the "parent code" will close the modal.Profit here: you do not have to guess whether
window.parent.Joomla.blabla
even exists.(btw, similar approach would be good as replacement for
window.parent.JUserSelect()
,jSelectArticle()
and for other similar trap)but
If we go the
callback
wayJoomla.currentModal.get().close()
, then we need small improve here.as @dneukirchen already wrote in Glip.
It better to have
Joomla.Modal
object, which can be easily extended in future.(btw, to support "multi modals"
current
can be transformed to array of opened modals andJoomla.Modal.getCurrent()
will return only last item from array.)Other notes, 2 potential issue here:
.open()/.close()
methods, wich doing something else