Repository navigation
feat(taskflow): pass consumer-owned feedback field for display messages through on handles - #279
Conversation
…es through on handles
…es through on handles
…tion-feedback' of https://github.com/chathura-de-silva/NSW-Core into feat/250-handle-declared-per-state-messages-for-post-action-feedback
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to The change adds optional consumer-defined handle messages, preserves them when provided, and omits them when absent. The documentation explains JSON encoding effects; no merge-blocking risk is identified. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review details
🚥 Pre-merge checks | ✅ 5
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
sthanikan2000
left a comment
sthanikan2000
left a comment
There was a problem hiding this comment.
Small suggestion. Wait for the review from @mushrafmim before making the changes.
| Command string `json:"command"` | ||
| Label string `json:"label"` | ||
| Element string `json:"element,omitempty"` | ||
| Messages json.RawMessage `json:"messages,omitempty"` |
There was a problem hiding this comment.
Shall we enforce the type more specific?
| Messages json.RawMessage `json:"messages,omitempty"` | |
| Messages map[string]HandleMessage `json:"messages,omitempty"` |
We can define the Message struct ads follows:
type HandleMessage struct {
Text string `json:"text"`
Variant string `json:"variant,omitempty"`
}There was a problem hiding this comment.
Shall we enforce the type more specific?
We can define the
Messagestruct ads follows:type HandleMessage struct { Text string `json:"text"` Variant string `json:"variant,omitempty"` }
I intentionally avoided doing this to allow the consumer to decide the format and keep core away from deciding what is/what type the feedback format is. Will do if needed (with Mushrafs feedback as you said.)
There was a problem hiding this comment.
This make sense. Downstream can define what's the config they willing to have. Let's wait for review from others.
…lared-per-state-messages-for-post-action-feedback
The docs said a handle's messages reaches the view exactly as written. The renderer marshals the view with encoding/json, which compacts whitespace and escapes <, > and & in a RawMessage, so only the parsed value is preserved. Both docs now say that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…es through on handles
The docs said a handle's messages reaches the view exactly as written. The renderer marshals the view with encoding/json, which compacts whitespace and escapes <, > and & in a RawMessage, so only the parsed value is preserved. Both docs now say that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3ddb31a to
4449291
Compare
…tion-feedback' of https://github.com/chathura-de-silva/NSW-Core into feat/250-handle-declared-per-state-messages-for-post-action-feedback
…lared-per-state-messages-for-post-action-feedback
…lared-per-state-messages-for-post-action-feedback
…lared-per-state-messages-for-post-action-feedback
…lared-per-state-messages-for-post-action-feedback
3 participants
Summary
Lets a render config attach post-action feedback to a button.
HandleClaimgains an optionalmessagesfield (json.RawMessage) that core carries fromrender.jsoninto the rendered view unchanged. Core knows what the field is for but not what it looks like. Each consumer defines the shape, so portals can stop inferring "what happened" from the task state after an action.Type of Change
Changes Made
taskflow/renderer/zoneview/zoneview.go: addMessages json.RawMessage \json:"messages,omitempty"`toHandleClaim`, and document it as opaque and owned by the consumer. The assembler logic is unchanged because legal handles are already copied whole into the view.taskflow/renderer/zoneview/renderer_test.go: addTestRender_PassesHandleMessagesThrough. It checks that a legal handle'smessagesreaches the view exactly as written, and that a handle withoutmessagesemits nomessageskey.taskflow/docs/template-reference.md: document the handle fields, including the optional pass-throughmessages.taskflow/docs/frontend-guide.md: note thatmessagesappears only when configured, and that the frontend defines its shape.Testing
Checklist
Related Issues
Closes #250
Related to OpenNSW/nsw-srilanka#550
Screenshots/Demo
N/A
Additional Notes
The field is optional and left out of the output when it isn't set, so existing render configs and consumers are unaffected. For reference, the trader-app's shape is keyed by task state with
{ text, variant }entries, and it is defined and documented in OpenNSW/nsw-srilanka#550, not here.Summary by CodeRabbit