Add `<HtmlPanel>` component #4

Asha20 · 2023-11-03T09:11:06Z

This PR adds the <HtmlPanel> component.

I've also updated the example app to include an HTML panel so you can see it in action.

eteeselink

First half of my review, second half coming up

eteeselink · 2023-11-03T10:14:52Z

README.md

@@ -274,13 +274,28 @@ Accepted props:
  loading
 - `chatboxRef` (resp. `inboxRef`, `popupRef`) - Pass a ref (created with `useRef`) and it'll be set to the vanilla JS [Chatbox](https://talkjs.com/docs/Reference/JavaScript_Chat_SDK/Chatbox/) (resp. [Inbox](https://talkjs.com/docs/Reference/JavaScript_Chat_SDK/Inbox/), [Popup](https://talkjs.com/docs/Reference/JavaScript_Chat_SDK/Popup/)) instance. See [above](#using-refs) for an example.
 - All [Talk.ChatboxOptions](https://talkjs.com/docs/Reference/JavaScript_Chat_SDK/Session/#ChatboxOptions)
+- `children?: ReactNode` - Optional. You can provide an `<HtmlPanel>` component as a child to use [HTML Panels](https://talkjs.com/docs/Features/Customizations/HTML_Panels/).


I think this should explicitly say that HtmlPanels can be the only direct children

eteeselink · 2023-11-03T10:17:57Z

README.md

+
+Accepted props:
+
+- `url: string` - The URL you want to load inside the HTML panel. The URL can be absolute or relative. We recommend using same origin pages to have better control of the page. Learn more about HTML Panels and same origin pages [here](https://talkjs.com/docs/Features/Customizations/HTML_Panels/)


This could be more precise, “to have better control” is a bit meaningless and also kinda wrong. It must be the same origin if they want to render children into it. If they don’t want to render their contents right here from React then the origin doesn’t matter. (Btw make sure you test both cases, ie that if there’s no children passed, and the panel has a different origin, there are no errors).

eteeselink · 2023-11-03T10:19:40Z

README.md

+
+- `height?: number` - Optional. The panel height in pixels. Defaults to `100px`.
+
+- `show?: boolean` - Optional. Sets the visibility of the panel. Defaults to `true`.


Worth explaining how this is different vs conditionally rendering the HtmlPanel element

eteeselink

love it. got lots of nits tho cause i'm like that

eteeselink · 2023-11-03T11:28:14Z

lib/HtmlPanel.tsx

+
+    return () => {
+      if (state.type === "loaded") {
+        state.panel.destroy();


shouldn't we always destroy it, regardless of loading state? else if customer code creates it and then quickly removes it, the chatbox will create it anyway.

(maybe just put the Promise<HtmlPanel> in the state, and do state.panelPromise.then(panel => panel.destroy()). i don't think there's a downside to already-resolved panels being destroyed in the next microtask instead of right there in the cleanup callback, right?

eteeselink · 2023-11-03T11:34:07Z

lib/HtmlPanel.tsx

+        state.panel.hide();
+      }
+    }
+  }, [state, show]);


shouldn't this be state.type instead of the entire state? (same for height)

lib/HtmlPanel.tsx

eteeselink · 2023-11-03T11:35:23Z

lib/HtmlPanel.tsx

+    <>
+      {state.type === "loaded" &&
+        createPortal(children, state.panel.window.document.body)}
+    </>


magic, isn't it? :D

btw i think this needs some "if no children given, then no portal" type logic, so we dont get errors when people mount non-same-origin html panels (eg because they're SSR'ing something from somewhere else)

eteeselink · 2023-11-03T11:41:44Z

lib/HtmlPanel.tsx

+      const panel = await box.createHtmlPanel({
+        url,
+        conversation: conversationId,
+        height,
+        show,
+      });


meta comment, I just realized that a set of <HtmlPanel> components in a react app is semantically equivalent to the htmlPanelOptions we pass from the JSSDK in the frontend.

this is actually super nuts. we internally have a declarative data structure, we built an imperative interface around that to mutate that data structure (createHtmlPanel, destroy, show, etc), and now here we're building the same thing in the opposite direction!

i don't think we need to do anything rn with this observation but i'm thinking that maybe one day we should deprecate all create* and set* methods entirely in favour of some vanilla JS, react-y declarative "spec your chatbox here" interface. would make the entire react sdk a near no-op, also.

eteeselink

Love it. I think I reviewed everything now!

eteeselink · 2023-11-13T16:36:50Z

example/panel.html

+  <head>
+    <meta charset="UTF-8" />
+    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+    <title>Document</title>


This is a bit meaningless, no? Maybe just keep the title out

eteeselink · 2023-11-13T16:39:35Z

example/panel.html

+    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+    <title>Document</title>
+    <style>
+      body {


Maybe let's just give body, html a padding: 0 for good measure? (and eg a margin on the button). i expect anybody will want to do that, so the html panel behaves pretty much like a div.

Not sure what you mean here; the buttons already have margin: 0.6rem auto

eteeselink · 2023-11-13T16:41:17Z

example/panel.html

+    <meta charset="UTF-8" />
+    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+    <title>Document</title>
+    <style>


How about we prepend something here like

<link rel="stylesheet" href="./your-styles.css">

lib/MountedBox.tsx

Previously we'd incorrectly pass their `children` into `useUIBox` which would cause the UIBoxes to be destroyed and re-initialized over and over.

lechinoix · 2024-01-18T11:52:06Z

Hello there ! Thank you so much for your Pull Request, I'll move to your fork as I definitely need it :) Might be nice to merge it at some point

lechinoix · 2024-01-18T11:59:02Z

@eteeselink Arf actually I cannot manage to use this branch directly, no types etc... Could you release it on a canary version or whatsoever ? It would be of great help :)

Asha20 · 2024-01-19T10:19:24Z

The experimental <HtmlPanel> component is now available by installing the @talkjs/react library with the next tag:

npm install @talkjs/react@next

Merge branch 'master' into feat/htmlpanel

(Fix) HtmlPanel crash bug

Asha20 added 3 commits November 3, 2023 09:00

feat: Add <HtmlPanel> component

ffed3f0

chore: Update package.json

914719e

chore: Fix import

7cfa463

eteeselink reviewed Nov 3, 2023

View reviewed changes

eteeselink requested changes Nov 3, 2023

View reviewed changes

eteeselink requested changes Nov 13, 2023

View reviewed changes

Asha20 added 5 commits November 29, 2023 07:42

wip

5253bb7

fix: Panels not being destroyed properly on component unmount

2cde9eb

feat: Validate children of UIBox components

cbdf35b

refactor: Apply review comments

faf5bdc

fix: Inbox and Popup re-rendering needlessly

ac6d3ff

Previously we'd incorrectly pass their `children` into `useUIBox` which would cause the UIBoxes to be destroyed and re-initialized over and over.

Asha20 added 2 commits January 18, 2024 13:36

Merge branch 'master' into feat/htmlpanel

7367fd3

chore: v0.1.6-beta.0

e66cdb9

Asha20 and others added 3 commits February 7, 2024 17:36

chore: v0.1.7-beta.0

359bb0f

Merge branch 'master' into feat/htmlpanel

Merge branch 'master' into feat/htmlpanel

f30cacf

chore: v0.1.9-beta.0

045d7af

Asha20 force-pushed the feat/htmlpanel branch from 6c92675 to 045d7af Compare June 19, 2024 06:57

Asha20 added 4 commits October 4, 2024 08:38

fix(htmlpanel) Properly reeinitialize session when user ID changes

b74d9d3

fix(htmlpanel) Reinitialize session when auth params change

92231d7

Merge pull request #13 from talkjs/fix/htmlpanel_crash

6217f1d

(Fix) HtmlPanel crash bug

chore: v0.1.9-beta.1

9c1cf8b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `<HtmlPanel>` component #4

Add `<HtmlPanel>` component #4

Asha20 commented Nov 3, 2023

eteeselink left a comment

eteeselink Nov 3, 2023

eteeselink Nov 3, 2023

eteeselink Nov 3, 2023

eteeselink left a comment

eteeselink Nov 3, 2023

eteeselink Nov 3, 2023

eteeselink Nov 3, 2023

eteeselink Nov 3, 2023 •

edited

Loading

eteeselink Nov 3, 2023

eteeselink left a comment

eteeselink Nov 13, 2023

eteeselink Nov 13, 2023

Asha20 Nov 30, 2023

eteeselink Nov 13, 2023

lechinoix commented Jan 18, 2024

lechinoix commented Jan 18, 2024

Asha20 commented Jan 19, 2024


		Accepted props:

		- `url: string` - The URL you want to load inside the HTML panel. The URL can be absolute or relative. We recommend using same origin pages to have better control of the page. Learn more about HTML Panels and same origin pages [here](https://talkjs.com/docs/Features/Customizations/HTML_Panels/)


		- `height?: number` - Optional. The panel height in pixels. Defaults to `100px`.

		- `show?: boolean` - Optional. Sets the visibility of the panel. Defaults to `true`.

Add <HtmlPanel> component #4

Are you sure you want to change the base?

Add <HtmlPanel> component #4

Conversation

Asha20 commented Nov 3, 2023

eteeselink left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eteeselink left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eteeselink Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eteeselink left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lechinoix commented Jan 18, 2024

lechinoix commented Jan 18, 2024

Asha20 commented Jan 19, 2024

Add `<HtmlPanel>` component #4

Add `<HtmlPanel>` component #4

eteeselink Nov 3, 2023 •

edited

Loading