-
Notifications
You must be signed in to change notification settings - Fork 1
MOC-204: chat, pass dom to extension via messaging #170
Conversation
WalkthroughWalkthroughThe changes involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Iframe
participant Document
User->>Iframe: Send "CHAT" message
Iframe->>Document: Parse current HTML
Document-->>Iframe: Return parsed HTML
Iframe->>User: Send JSON representation
User->>Iframe: Send "CHAT_NEW_MESSAGE"
Iframe->>User: Acknowledge with "ok"
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/mocksi-lite-next/src/pages/content/mocksi-extension.tsx (5 hunks)
Additional comments not posted (4)
apps/mocksi-lite-next/src/pages/content/mocksi-extension.tsx (4)
43-49
: LGTM!The changes to the
getIframeStyles
function look good. The new "DEMO_ACTIVE" case handles the iframe display properties appropriately for what seems to be a demo mode.Also applies to: 65-69
75-110
: LGTM!The new
innerHTMLToJson
function looks great! It provides a handy utility for converting HTML strings to JSON format. The recursive approach ensures that the entire document structure is captured accurately. The use ofDOMParser
is appropriate, and the function follows a clear and logical structure.
154-160
: LGTM!The changes to the message listener for handling the "CHAT" message look good. When a "CHAT" message is received, it appropriately converts the current document's body into JSON format using the
innerHTMLToJson
function and sends the data back in the response. This functionality aligns with the goal of passing the DOM to the extension via messaging.
122-122
: LGTM!The addition of the
flags
parameter to thefindReplaceAll
function is a good enhancement. It allows for specifying flags or options for the find and replace operation, providing more flexibility and customization. Theflags
parameter is correctly included in thecontent
property of the modification object, ensuring that the flags are passed along with the find and replace patterns.Also applies to: 171-171
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.
Just a question for @jonathankap
default: | ||
return {}; | ||
} | ||
} | ||
|
||
export const innerHTMLToJson = (innerHTML: string): string => { |
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.
@jonathankap I kinda feel this should be part of Reactor. Thoughts?
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.
We're all thinking the same thing! 🤣 It's already there:
HARlighter/packages/reactor/reactor.ts
Line 125 in c947a60
exportDOM(element: null | HTMLElement = null): DomJsonExportNode[] { |
@fitzk please update to use the reactor call instead
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.
Update to use reactor to create the JSON
default: | ||
return {}; | ||
} | ||
} | ||
|
||
export const innerHTMLToJson = (innerHTML: string): string => { |
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.
We're all thinking the same thing! 🤣 It's already there:
HARlighter/packages/reactor/reactor.ts
Line 125 in c947a60
exportDOM(element: null | HTMLElement = null): DomJsonExportNode[] { |
@fitzk please update to use the reactor call instead
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/mocksi-lite-next/src/pages/content/mocksi-extension.tsx (1 hunks)
Additional comments not posted (1)
apps/mocksi-lite-next/src/pages/content/mocksi-extension.tsx (1)
118-130
: LGTM!The new message handling logic for "CHAT" and "CHAT_NEW_MESSAGE" messages looks good. The changes are consistent with the PR objective of passing DOM to the extension via messaging.
For the "CHAT" message, the response correctly includes the current DOM state exported by
reactor.exportDOM()
, allowing the DOM to be passed to the extension. The "CHAT_NEW_MESSAGE" message handling also looks fine.Overall, the code changes in this segment are approved.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/mocksi-lite-next/src/pages/content/mocksi-extension.tsx (3 hunks)
Additional comments not posted (3)
apps/mocksi-lite-next/src/pages/content/mocksi-extension.tsx (3)
42-51
: LGTM!The code changes are approved.
127-132
: LGTM!The code changes are approved.
162-169
: LGTM!The code changes are approved.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/mocksi-lite-next/src/pages/content/mocksi-extension.tsx (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/mocksi-lite-next/src/pages/content/mocksi-extension.tsx
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.
Nice!
Let's get these merged so Jon can merge in his PR's and I will follow with one more PR to add chat history to the UI
https://github.com/Mocksi/nest/pull/45
To trigger the chat message you just have to edit a demo, then click the chat icon on the toolbar, from there you should see the message from the service worker in the console.
Summary by CodeRabbit
Summary by CodeRabbit