V8: Move AngluarJS event names into a constants file to DRY it up.

[PeteDuncanson]

The eventsService can emit a lot of different events but I can't seem to find a definitive list of the events it can fire. I've logged this as an issue on the docs site here (umbraco/UmbracoDocs#1595).

However it got me thinking that we probably shouldn't be using magic strings for these events. Without a definitive list its easy to mistype an event or select the wrong one which could cause all sorts of issues. I suggest we find a list of all the events of note and add them to a constants file and then allow controllers to DI these in and reference them from one place. Should greatly reduce the amount of magic strings and some duplication of events too no doubt.

Redux uses this idea for its "actions" that your code can listen out for (https://redux.js.org/basics/actions). All they are is constants with the magic string stored in them. In AngularJS we would need to put this in a module or unit of code you could pull in via DI. Its best practise to have your actions in a separate file and import them so that everything is referring to the same action. It also gives you one place to go look to see what is available and to add new ones which helps keep everything "sane".

I'm assuming packages etc. will be exempt they can roll their own but there is no reason Core can't go this route. The magic strings will still work which means we can swap them out bit by bit and make it all a bit more robust :)

I'd expect it to work a little like this:

Taken from:

function navigationService($routeParams, $location, $q, $timeout, $injector, eventsService, umbModelMapper, treeService, appState) {

    //the promise that will be resolved when the navigation is ready
    var navReadyPromise = $q.defer();

    //the main tree's API reference, this is acquired when the tree has initialized
    var mainTreeApi = null;

    eventsService.on("app.navigationReady", function (e, args) {
        mainTreeApi = args.treeApi;
        navReadyPromise.resolve(mainTreeApi);
    });

Becomes:

// NOTE: We now inject in "eventNames" - I'm open to offers on the naming of this
function navigationService($routeParams, $location, $q, $timeout, $injector, eventsService, eventNames, umbModelMapper, treeService, appState) {

    //the promise that will be resolved when the navigation is ready
    var navReadyPromise = $q.defer();

    //the main tree's API reference, this is acquired when the tree has initialized
    var mainTreeApi = null;

    eventsService.on( eventNames.APP_NAVIGATION_READY, function (e, args) {
        mainTreeApi = args.treeApi;
        navReadyPromise.resolve(mainTreeApi);
    });

One issue I've seen while poking around with this is files like this which have nested state etc. Will need a little thought https://github.com/umbraco/Umbraco-CMS/blob/91c52cffc8b7c70dc956f6c6610460be2d1adc51/src/Umbraco.Web.UI.Client/src/common/services/appstate.service.js

[PeteDuncanson]

Had a thought about this. Actions in Redux work really well as you get intellisense in VSCode as they are "required" at the top of any file you want to use them in so it just works. As AngularJS uses it custom DI implementation I'm not sure if VSCode et al can do its magic with that. I've found this article for getting intellisense working but I think its only the built in AngularJS stuff and not linking files together but I'll have a play and see what its like.

https://devblogs.microsoft.com/typescript/the-future-of-declaration-files-2/

In short you just install this globally and VSCode should pick it up.

https://www.npmjs.com/package/@types/angular

Does raise an interesting issue. Could we create a custom type definition file for Umbraco js goodies and get intellisense?

[PeteDuncanson]

Ok so I've been having a chat with @base33 and we are both keen to get the code base using TypeScript which would allow us to very easily use enums (string ones) for the event names and we can pull those in. That would give us type safety and ease of documentation. Think this would be the way to go with this one.

[PeteDuncanson]

See also #5215

[Shazwazza]

So shall i close this one then @PeteDuncanson ?

[PeteDuncanson]

@Shazwazza I think so as I'll add it to one of my tasks to do for #5215 :)

[PeteDuncanson]

Well...given #5215 does that mean this issue gets reopened?

[umbrabot]

Hiya @PeteDuncanson,

Just wanted to let you know that we noticed that this issue got a bit stale and might not be relevant any more.

We will close this issue for now but we're happy to open it up again if you think it's still relevant (for example: it's a feature request that's not yet implemented, or it's a bug that's not yet been fixed).

To open it this issue up again, you can write @umbrabot still relevant in a new comment as the first line. It would be super helpful for us if on the next line you could let us know why you think it's still relevant.

For example:

@umbrabot still relevant
This bug can still be reproduced in version x.y.z

This will reopen the issue in the next few hours.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂