-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add event parameter manipulation to ActionSendMessageWidget #9528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Extends the action-sendmessage widget to support passing event properties to the dispatched message params object New features: - $event-* attributes: Add properties directly to params/event object (e.g., $event-type="tm-navigate" adds type: "tm-navigate" to params) - $eventNames and $eventValues: Filter-based property assignment to params object (e.g., $eventNames="[enlist<list-event>]") This enables action-sendmessage to forward event properties from MessageCatcherWidget and other event sources while maintaining backward compatibility with existing functionality.
Extends event handling to preserve types when forwarding events between widgets by using JSON serialization. MessageCatcherWidget changes: - Exports new "json-event" variable containing the full event object as JSON, preserving boolean and number types SendMessageWidget changes: - Accepts new $eventJson attribute for JSON-serialized event data - Parses JSON and merges properties into dispatched params object This enables type-safe event forwarding while maintaining backward compatibility and allowing flexible event manipulation.
✅ Deploy Preview for tiddlywiki-previews ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Confirmed: yaisog has already signed the Contributor License Agreement (see contributing.md) |
📊 Build Size Comparison:
|
| Branch | Size |
|---|---|
| Base (master) | 2448.8 KB |
| PR | 2451.8 KB |
Diff: ⬆️ Increase: +3.0 KB
✅ Change Note Status
All change notes are properly formatted and validated!
📝 $:/changenotes/5.4.0/#9528
Type: enhancement | Category: widget
Release: 5.4.0
Add event parameter manipulation to ActionSendMessageWidget
🔗 #9528
👥 Contributors: yaisog
📖 Change Note Guidelines
Change notes help track and communicate changes effectively. See the full documentation for details.
|
I am not sure if the might not work as expected if there are boolean types involved, e.g. |
|
Thanks @yaisog this is looking good. It would be helpful to document some of the common use cases (like forwarding a message). |
|
@yaisog this is a much needed improvement, thank you. I think we should drop the It is also very unfortunate that we have used the term "events" in conjunction with Widget Messages as that often leads to confusion with DOM events. We should take the opportunity in 5.4 to start deprecating parameters and variables to do with Widget messages that use the |
yaisog
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @saqimtiaz, I thankfully removed the $event* attributes and also renamed the other to $messageJson (or should it be $messageJSON?).
I also amended the MessageCatcherWidget to set message-* and message-paramObject variables in addition to the current event variables, preparing for deprecation and later removal of the latter.
I think regarding messages there is not a lot else to clean up.
Next up I'll work over the documentation regarding MessageCatcherWidget and ActionSendMessageWidget to reflect the changes and maybe add some examples.
messageJSON would be preferrable, thank you. |
|
I pushed an update that implements quite comprehensive documentation tiddlers for ActionSendMessageWidget and MessageCatcherWidget that should satisfy @Jermolene. Note that I also took the opportunity to switch the hierarchy between [[Messages]] and [[Core Messages]], so that the latter is now a direct descendant of the former, not vice versa. This made it necessary to update the tags on all tm-* doc tiddlers, hence the large number of changed files. |
|
@yaisog the plan for v5.4.0 is to also introduce a similar JSON blob to provide by the eventcatcher widget. The PR is still a draft and needs a few small changes. I am wondering if the code that copies the object so it can be safely strinfigied could be reused here, or if we could refactor to allow the same code to be usable in both places. |
|
Hi @saqimtiaz, that's a good idea to reuse that code. From a first look I'd say it should be usable as is. It does have a few more safeguards than we will probably need here, but that shouldn't hurt. |
|
Hi @saqimtiaz, I had a closer look and I don't think it'll work.
I'd have to add a parameter to copyObjectPropertiesSafe() to switch it into a different mode that mostly copies the code that is now in messagecatcher.js and calls that when the parameter is set. Not sure if that makes sense. Let me know your thoughts on this. |
|
@yaisog there might be room to standardize on an implementation for both widgets that when copying the object properties, entirely skips DOM elements, Widgets and circular references instead of reporting them as strings, as these would not be of value in either scenario. |
|
Hi @saqimtiaz, I'll happily take a look. How should we handle the event object, which is not relevant for the $messagecatcher? We could have a function parameter like |
|
@yaisog do you mean |
… conversion This commit uses the $tw.utils.copyObjectPropertiesSafe function from PR TiddlyWiki#8919 to gather the properties from the message event object before converting them to JSON. This ensures that only safe and relevant properties are included in the JSON representation, avoiding potential issues with circular references or unwanted data. The function was modified to silently skip DOM elements, widgets or unserializable properties and extended with an exception list to allow specific properties to also be skipped.
Yes, exactly that. I took your copyObjectPropertiesSafe method and implemented it here. Instead of outputting the unusable elements as "[Reason]", they are now skipped silently. to remove Let me know if that works for your EventCatcherWidget implementation or if we should fine-tune. |
|
@yaisog at first glance I think that looks good. Perhaps just add a short comment block before the new utils method explaining the structure of the optional options object. If there are any minor adjustments required to this method, I can work on them when I get the chance to pick up the work on the eventcatcher PR. We should take this opportunity to standardize as much as we can in terms of how both catcher widgets provide variables to the actions they invoke. |
I agree 100%. I will add a comment block above the method with a short explanation. |
|
@yaisog having given this some further thought, I do not think we should be excluding the browser DOM event that initiated the message (event.event) from the JSON payload that we provide. The folllowing properties can potentially be useful in $messagecatcher:
|
|
Hi @saqimtiaz, just getting back to this. I see your point. But if we want to leave these in, we have to leave the whole In that case I can remove the exclude list from the |
|
Hi @yaisog I think it makes sense to provide access to the stringified version of the event object in messagecatcher, and the version of However, I am less sure about the changes to I don't have the time to look at this carefully at the moment, but I am concerned that re-emitting a caught message needs to behave exactly the same as the original message if we can help it. One alternative here is a passthrough option on the messagecatcher widget, where it invokes actions and allows the original message to propagate. One can imagine a widget message whose handling also requires access to the original event object, and we should be able to catch and re-emit it. |
|
Hi @saqimtiaz,
OK, I'll have a look when that is merged and then remove that file from the PR.
I did propose that back in #6493. However, @Jermolene favored extending the ´<$action-sendmessage>
Can you name any message that uses the |
This PR extends the
$action-sendmessagewidget to support passing and manipulating event properties, enabling better integration with$messagecatcherand more flexible event handling.Currently, when using the MessageCatcherWidget to capture messages like
tm-navigate, there's no straightforward way to fully forward those events with the ActionSendMessageWidget since it requires certain properties of theeventobject to be set, e.g.navigateTo,navigateFromTitleor (boolean)navigateSuppressNavigation. The MessageCatcherWidget converts all event properties to strings for the event-* variables, which loses type information.This PR enables type-preserving event forwarding via JSON serialization. String properties may also be set using single-parameter attributes and / or filter expressions for multiple parameters (similar to the existing
$namesand$valuesattributes).$tw.utils.copyObjectPropertiesSafe()for JSON generationChanges
MessageCatcherWidget
json-messagevariable containing primitive message properties (string, boolean, number) as JSONevent-*variablesevent-*variables asmessage-*variables and deprecates theevent-*versionsActionSendMessageWidget
Added support for injecting properties into the dispatched params object:
$messageJSON: Accepts JSON-serialized event data, parsing and merging properties with preserved typesPriority order (later overwrites earlier):
$eventJson$eventNames/$eventValues$event-*attributestypefrom$message(if specified)Usage Examples
Simple event forwarding with additional actions:
Resolves #7343
Resolves #6493