-
Notifications
You must be signed in to change notification settings - Fork 17
chore: Migrate front-end logic to React #139
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: main
Are you sure you want to change the base?
Conversation
Custom Hook (useChatState) * Pure state management: All chat state logic centralized * Typed methods: appendMessage, appendMessageChunk, clearMessages, etc. * No DOM dependencies: Can be tested independently Clean Component Architecture * ChatInput: Uses callback pattern to expose methods (onMethodsReady) * ChatContainer: Uses custom hook + listens for CustomEvents * No imperative refs: All communication through props and events Shiny Integration Ready * Event-driven: Components listen for shiny-chat-* CustomEvents * Type-safe: All event details properly typed * Simple wrapper: Future Shiny component just dispatches events
// Message operations (for Shiny integration) | ||
appendMessage: (message: Message) => void | ||
appendMessageChunk: (message: Message) => void | ||
clearMessages: () => void | ||
removeLoadingMessage: () => void | ||
updateUserInput: (update: UpdateUserInput) => 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.
Seems like we could take some inspiration from #105 and re-work/simplify the event system?
handleWillContentChange = () => { | ||
ShinyMarkdownStreamOutput.#doUnBind(this) | ||
} | ||
|
||
// Public callback methods for React component and testing | ||
handleContentChange = () => { | ||
// Render Shiny HTML dependencies and bind inputs/outputs | ||
if (this.streaming) { | ||
ShinyMarkdownStreamOutput._throttledBind(this) | ||
} else { | ||
ShinyMarkdownStreamOutput.#doBind(this) | ||
} | ||
|
||
// Callback for when content changes - can be used for custom logic | ||
this.dispatchEvent( | ||
new CustomEvent("contentchange", { | ||
detail: { content: this.content }, | ||
}), | ||
) | ||
} |
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.
Feels like the binding logic could be moved into the MarkdownStream
component implementation?
if (contentType === "text") { | ||
return <div dangerouslySetInnerHTML={{ __html: processedContent }} /> | ||
} else if (contentType === "html") { | ||
return <div dangerouslySetInnerHTML={{ __html: processedContent }} /> |
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 could do something closer to what 'react-markdown'
does (parses string into hast tree, then creates a JSX runtime from that) https://github.com/remarkjs/react-markdown/blob/fda7fa560bec901a6103e195f9b1979dab543b17/lib/index.js#L348
No description provided.