User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Place all the functions from guidedtours.es6.js into a an object "Joomla.guidedTour" so instead of calling getTourInstance() we would now call Joomla.guidedTour.getTourInstance() instead.
This will help avoid function name clashes, and is a step towards making the code more extensible.
Run the tours included in Joomla 5
The tours run as expected
The tours run as expected
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed
Category | ⇒ | JavaScript Repository NPM Change |
Status | New | ⇒ | Pending |
Title |
|
I do agree with the arguments of @dgrammatiko. So @GeraintEdwards what is your use case? Why do you need to make it extensible? Perhaps we need to find a way that extensions can interact with the functionality of guided tours which is ES module compatible.
@dgrammatiko I have been looking at this javascript mainly in the webbrowser to figure out how it was working and it was loading the es5 version as 'nomodule' each time I checked (latest version of Firefox in Linux). The really bizarre thing is that at some point this morning when I was rebuilding the scripts it switched to 'module' and I then tried and tried to recreate the nomodule behaviour but couldn't.
I was just about to post this response and I see it again
<script src="/media/plg_system_guidedtours/js/guidedtours-es5.min.js?b1c9af93e530977e63de06083aca3fea" nomodule="" defer=""></script>
Something very strange going on :(
Nevertheless I would argue that these methods need to be exported/made available in a global object to allow them to be interacted with.
@laoneo I can see a few scenarios where we may wish to extend or modify the behaviour of these methods.
My idea is to possibly add 'awaihandler', 'stephandler', 'prehandler', 'posthandler', 'choicehandler' type capability so that when steps and tours are loaded/navigated that these methods (if set for the relevant step) are called by the script.
Additionally the code is peppered with switch and "if/else if" statements based on element types or step types . It would be good, in my mind, to make this more object oriented.
At present its not possible to customise the handling of certain methods let alone implement custom step types or element handling.
I don't think the core should have a really complicated guided tour system by default but it should make it possible to allow 3PD or training providers to implement more advanced tours if they wish.
Something very strange going on :(
nothing strange happening, Joomla renders both scripts (with type=module and nomodule) but any browser >2018 will use only the type=module
Nevertheless I would argue that these methods need to be exported/made available in a global object to allow them to be interacted with.
actually the script itself does not expose anything by design. There’s no api to extend anything. If you want to customize this script Joomla already provides a way: use an override and write your own take. About global objects: that’s not how we code anymore in 2023, neither in js nor in php…
Thanks for your input.
Are you suggesting the best way to extend this functionality is to override the whole file via the WebAssetManager? TBH I've been stuck having to support 'dead' browsers for too long and not ever expecting to be able to use more advanced JS any time soon :( Sorry for the miss-understanding.
I can see how we we load our own version of the js in place of the preset assets but replacing the whole file feels like overkill. If the built in file was 'class based' we could 'inherit' the base methods or the existing functions were exported then an override script would only override the methods that need changing or could re-use the exported functions. Did I miss something else in how modules work?
What do think of my point about reducing the use of switch and "if/else if" statements?
TBH I've been stuck having to support 'dead' browsers for too long and not ever expecting to be able to use more advanced JS any time soon :( Sorry for the miss-understanding.
FYI Joomla 5.0 will NOT support old/dead browsers once #39618 is merged.
but replacing the whole file feels like overkill.
In this particular case it's not as the API for the guided tours was not designed to be extended. Also providing your own file you could form the API/functionality the way you want. So without the need the core script to be needless more complicated you could achieve what you want right now, thus I don't see how overengineering something (that won't provide anything to core apart that maintainers would have to maintain code that it won't be used anywhere in the core) is the way to go. Also overriding the whole file has another advantage: less code both in the server side and less scripts glued together on a global object, so probably a better design altogether...
if the built in file was 'class based' we could 'inherit' the base methods or the existing functions were exported then an override script would only override the methods that need changing or could re-use the exported functions
That's not how most of the JS should be extended (class inheritance). It's, mostly, preferred to be event based. Ie have the guided tours fire events and then depending on the step/input/whatever a specific event listener/handler will run
. But, guided tours was neither coded as a class and neither have an event bus/system in place. So back to overriding the whole file is the preferred way to go. Also, really, even if the class inheritance is the preferred way here (I doubt it), the class SHOULD NOT be a global object for many reasons...
Having it not a global object make sense, but having no events doesn't make sense, especially if joomla's main strange is the event system (in php) and the extensibility. Maybe it's not needed to directly extend the guidedTour object, having the possibility to prepare things before or after a step is execute make sense for me.
I'll shelve this PR for now and have a think about events, breaking out the handling of different DOM element types etc. to make the code more sustainable/maintainable.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-06-19 08:28:47 |
Closed_By | ⇒ | GeraintEdwards | |
Labels |
Added:
NPM Resource Changed
PR-5.0-dev
|
The first line of the script
import Shepherd from 'shepherd.js';
indicates that this is an ES Module script. On top of that the script is loaded AS module:joomla-cms/build/media_source/plg_system_guidedtours/joomla.asset.json
Line 29 in 4efb936
Please close it, you're not improving anything...