mike-jumper commented on pull request #573:
URL: https://github.com/apache/guacamole-client/pull/573#issuecomment-830426970
This is looking better, yes. My feedback on this latest iteration:
* It's seems odd to me to subclass `Guacamole.Event.Target` rather than
`Guacamole.Event`.
* For external code to be able to hook into the dispatched
`Guacamole.Event`, there will need to be some sort of global
`Guacamole.Event.Target` that extensions, etc. can rely on existing.
* It's a bit disconcerting to me that guaranteed is in quotes here:
> Any event handlers that return a Promise will be 'Guaranteed' to finish
before the logout refresh happens.
;)
I think it would make more sense to subclass `Guacamole.Event`, provided
some mechanism on that subclass to allow implementations to associate the
promises of their own async operations with the event, and then handle
`$q.all()` or similar in the part of the code that owns the event after
`dispatch()` has finished.
For example:
1. Magical async subclass of `Guacamole.Event` is dispatched via
`dispatch()`. Perhaps the subclass has an array of associated promises. Perhaps
it keeps that hidden and provides a function for associating operations.
Perhaps it exposes a single promise that can be replaced with another to form
an arbitrary chain. In any case, it provides some API for associating the
promises of additional async operations with the event.
2. A listener associates its own additional async operation promise with the
event using the above mechanism.
3. `dispatch()` returns and the code that originally invoked `dispatch()`
inspects the contents of that event and deals with the relevant promises.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
