feat(content-sidebar): Expand Box AI Sidebar to Modal #3878
Conversation
| onSelectAgent={onSelectAgent} | ||
| recordAction={recordAction} | ||
| shouldHideAgentSelectorOnLoad={false} | ||
| // @ts-ignore variant will be available in higher version |
There was a problem hiding this comment.
do we still need this comment?
There was a problem hiding this comment.
Yeah, it's no longer needed, I've removed it.
| @@ -16,16 +16,19 @@ export interface BoxAISidebarContextValues { | |||
| cache: { encodedSession?: string | null; questions?: QuestionType[] }; | |||
| contentName: string; | |||
| elementId: string; | |||
| fileExtension: string; | |||
There was a problem hiding this comment.
Curious why do we pass those props, especially avatar which will be deleted from modal and sidebar shortly?
There was a problem hiding this comment.
fileExtension is used in IntelligenceModal - it's used as one of the properties that are tracked with Resin (and Pendo). It will also be used to track the "load" of Box AI Sidebar, which will be added with a different ticket.
And we can't really remove the avatarURL until the avatar is removed from BoxAiContentAnswers.
| <> | ||
| <SidebarContent | ||
| actions={renderActions()} | ||
| className={classNames('bcs-BoxAISidebar', { 'with-modal-open': isModalOpen })} |
There was a problem hiding this comment.
suit naming would make the class is-modal-open rather than with-
https://github.com/suitcss/suit/blob/master/doc/naming-conventions.md#is-stateOfComponent
| // @ts-ignore variant will be available in higher version | ||
| variant="sidebar" | ||
| /> | ||
| <div className="bcs-BoxAISidebar-agent-selector-container"> |
There was a problem hiding this comment.
so for suit naming, the class name should start with the component name and then descendants are listed in camelcase after a dash: bcs-BoxAISidebarContent-agentSelector
but since bcs-BoxAISidebar is already the pattern in this file, I guess call this bcs-BoxAISidebar-agentSelector?
| expect(await screen.findByTestId('content-answers-modal')).toBeInTheDocument(); | ||
|
|
||
| const switchToSidebarButton = screen.getByRole('button', { name: 'Switch to sidebar view' }); | ||
| await userEvent.click(switchToSidebarButton); | ||
|
|
||
| expect(screen.queryByTestId('content-answers-modal')).not.toBeInTheDocument(); |
There was a problem hiding this comment.
is there a role or label we can query by instead of test id?
| const onModalClose = () => { | ||
| setIsModalOpen(false); | ||
| }; | ||
|
|
||
| const onSwitchToModalClick = () => { | ||
| setIsModalOpen(true); | ||
| }; |
There was a problem hiding this comment.
nit: these could be named as handleModalClose and handleModalOpen
reserving on for the prop and handle for the function
Thanks for review @tjuanitas. I will merge it, since it's a time sensitive change. We will work on UI improvements of Box AI Sidebar very soon and we'll address your comments then. |
Description:
Added option to Switch Box AI Sidebar view into Modal view. The session, conversation history and Agent Selector data are shared between the two, so that user can start conversation in one experience, switch and seamlessly continue in the other.
More detailed checks verifying the continuity of conversation will be added in E2E tests as a separate PR soon.
Demo:
