-
Notifications
You must be signed in to change notification settings - Fork 5
xmtp quick actions #245
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
xmtp quick actions #245
Conversation
| /** | ||
| * Individual action definition | ||
| */ | ||
| export type Action = { |
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.
rename to XmtpAction. It might be confusing to users with Action from eliza
| * Agents use this to present interactive button options to users | ||
| */ | ||
|
|
||
| export const ActionsContentSchema = z.object({ |
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.
XmtpActionsContentSchema
| z.object({ | ||
| id: z.string(), | ||
| label: z.string(), | ||
| imageUrl: z.string().url().optional(), | ||
| style: z.enum(['primary', 'secondary', 'danger']).optional(), | ||
| expiresAt: z.string().datetime().optional() | ||
| }) |
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.
why not create a schema and infer the type on line 22 from it
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.
also use nullish for everything
| if (!content.id || typeof content.id !== 'string') { | ||
| throw new Error('Actions.id is required and must be a string') | ||
| } | ||
|
|
||
| if (!content.description || typeof content.description !== 'string') { | ||
| throw new Error('Actions.description is required and must be a string') | ||
| } | ||
|
|
||
| if (!Array.isArray(content.actions) || content.actions.length === 0) { | ||
| throw new Error('Actions.actions is required and must be a non-empty array') | ||
| } | ||
|
|
||
| if (content.actions.length > 10) { | ||
| throw new Error('Actions.actions cannot exceed 10 actions for UX reasons') | ||
| } | ||
|
|
||
| // Validate each action | ||
| content.actions.forEach((action, index) => { | ||
| if (!action.id || typeof action.id !== 'string') { | ||
| throw new Error(`Action[${index}].id is required and must be a string`) | ||
| } | ||
|
|
||
| if (!action.label || typeof action.label !== 'string') { | ||
| throw new Error(`Action[${index}].label is required and must be a string`) | ||
| } | ||
|
|
||
| if (action.label.length > 50) { | ||
| throw new Error(`Action[${index}].label cannot exceed 50 characters`) | ||
| } | ||
|
|
||
| if (action.style && !['primary', 'secondary', 'danger'].includes(action.style)) { | ||
| throw new Error(`Action[${index}].style must be one of: primary, secondary, danger`) | ||
| } | ||
|
|
||
| if (action.expiresAt && !this.isValidISO8601(action.expiresAt)) { | ||
| throw new Error(`Action[${index}].expiresAt must be a valid ISO-8601 timestamp`) | ||
| } | ||
| }) | ||
|
|
||
| // Check for duplicate action IDs | ||
| const actionIds = content.actions.map((action) => action.id) | ||
| const uniqueActionIds = new Set(actionIds) | ||
| if (actionIds.length !== uniqueActionIds.size) { | ||
| throw new Error('Action.id values must be unique within Actions.actions array') | ||
| } | ||
|
|
||
| if (content.expiresAt && !this.isValidISO8601(content.expiresAt)) { | ||
| throw new Error('Actions.expiresAt must be a valid ISO-8601 timestamp') | ||
| } | ||
| } |
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.
why aren't we validating with zod?
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.
this is their code actually, so i didnt mess with it, its their library sort of, just copy pasted here
| * Content Type ID for Intent messages | ||
| * Following XIP-67 specification for inline actions | ||
| */ | ||
| export const ContentTypeIntent = new ContentTypeId({ |
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.
prefix with Xmtp
| * Intent content structure | ||
| * Users express their selection by sending Intent messages when they tap action buttons | ||
| */ | ||
| export const IntentContentSchema = z.object({ |
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.
prefix with Xmtp
| /** | ||
| * Validates Intent content according to XIP-67 specification | ||
| */ | ||
| private validateContent(content: IntentContent): void { |
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.
prefix this entire file with Xmtp
hishboy
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.
LGTM but please address comments
No description provided.