feat(intelligent-assistant): implement docked and overlay display modes for Notebook - #4093
Conversation
…es for Notebook Enable notebook functionality in docked and overlay (non-fullscreen) display modes. Previously, notebooks were only accessible in fullscreen/embedded mode. - Add activeNotebookId to drawer context for state management without routes - Show Chat/Notebooks tabs in overlay and docked modes - Manage notebook selection via state instead of URL navigation in compact modes - Add header actions (close, add document, toggle sidebar) for compact modes - Use single-column grid layout for notebook cards in narrow panels - Fix NotebookView flex chain for proper sizing within docked panels - Override PF Chatbot embedded CSS (min-height, overflow) to prevent content overflow in constrained containers - Make DrawerPanelContent take full width when in compact mode - Update tests for new context fields and tab visibility RHIDP-14656 Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
PR Summary by QodoEnable Notebook overlay/docked modes with state-based selection and compact header actions
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4093 +/- ##
==========================================
+ Coverage 58.30% 58.33% +0.03%
==========================================
Files 2427 2432 +5
Lines 96721 96966 +245
Branches 26915 27016 +101
==========================================
+ Hits 56390 56563 +173
- Misses 38869 38939 +70
- Partials 1462 1464 +2
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Code Review by Qodo
1.
|
…ss display mode switch Notebooks now work in overlay/docked modes, so shellViewTab should remain on Notebooks (1) when switching from embedded to overlay instead of resetting to Chat (0). Also adds changeset for the feature. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@rohitratannagar please add screen recordings. |
…bounds Signed-off-by: rohitratannagar <rohitratannagar2003@gmail.com>
|
cc: @ShiranHi Screen.Recording.2026-07-30.at.12.50.54.PM.mov |
|
@rohitratannagar looks good to me, thank you! I have a couple of minor points:
|
…ry, and scroll controls - Remove extra padding from compact modal dialogs for proper header alignment - Add !important to compact style overrides to win specificity against MUI defaults - Split body/bodyCompact styles so scroll controls work in fullscreen mode - Add flexWrap to notebooks header so title stays on one line in narrow panels - Add retry loop to loginAsGuest for cold-start guest auth reliability - Adjust scoped dialog margins for better compact panel centering Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
cc: @ShiranHi Screen.Recording.2026-07-30.at.5.48.12.PM.mov |
…ight and size Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
cc: @HusneShabbir |
HusneShabbir
left a comment
HusneShabbir
left a comment
There was a problem hiding this comment.
When accessing notebooks in any mode other than fullscreen, the chatbot header components (Close Notebook, Add, Expand Sidebar, and Display Options) should be hidden and non-interactive. This matches the behavior in the prototype, but it hasn't been implemented in the current PR.
Prototype:
Screen.Recording.2026-08-03.at.5.32.24.PM.mov
PR Local behaviour:
Screen.Recording.2026-08-03.at.5.33.07.PM.mov
HusneShabbir
left a comment
HusneShabbir
left a comment
There was a problem hiding this comment.
Also, making the chatbot header components (Close Notebook, Add, and Expand Sidebar) equal-sized would improve the visual consistency and make the UI look cleaner.
Prototype:
UI:
There was a problem hiding this comment.
The Expand Sidebar icon in Notebooks should mirror its direction when clicked. This behavior exists in the prototype but is missing in the current PR.
Prototype:
Screen.Recording.2026-08-03.at.5.48.09.PM.mov
PR Behaviour:
Screen.Recording.2026-08-03.at.5.48.34.PM.mov
…ode and mirror sidebar icon - Hide expand strip, top bar, and sidebar panel in overlay/docked modes - Mirror sidebar expand/collapse icon direction based on state - Add size prop to sidebar icons for compact header usage - Sync upload-in-progress, sidebar-collapsed, and modal-open state to parent - Disable Add button and dim header actions when upload modal is open Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@HusneShabbir — addressed your review feedback in the latest commits:
Additional improvements:
Could you take another look? Thanks! |
HusneShabbir
left a comment
HusneShabbir
left a comment
There was a problem hiding this comment.
/lgtm
All the changes look good, and the behavior closely resembles the prototype.
HusneShabbir
left a comment
HusneShabbir
left a comment
There was a problem hiding this comment.
Not sure if the prototype needs an update, but I noticed one difference. In the notebook fullscreen mode, when the sidebar is collapsed, our implementation (PR #4093) still shows the Add Resources button, whereas the prototype hides it along with the sidebar.
Screen.Recording.2026-08-04.at.7.57.03.PM.mov
cc: @ShiranHi
Prototype need to be updated. |
|
I've updated the prototype (VPN required) to match the implementation. The Add Resources button now remains visible when the resource list is collapsed in fullscreen mode. |
There was a problem hiding this comment.
Overall looks good, I have few observations on UI and some code-wise improvements.
Couple of issues on the confirmation modal:
- Modal on top of a modal is not good. I know this isn't introduced by this PR but it is a easy win in my opinion to momentarily close the Upload modal behind before opening a confirmation modal and when user completes the interaction in confirmation modal we can bring back the add modal or directly upload the files after confirmation.
- Overwrite modal styles needs some update, at least the modal header seems off.
| const [sidebarCollapsed, setSidebarCollapsed] = useState(isCompact); | ||
| const [isUploadModalOpen, setIsUploadModalOpen] = useState(false); | ||
|
|
||
| useImperativeHandle(ref, () => ({ | ||
| openUploadModal: () => setIsUploadModalOpen(true), | ||
| toggleSidebar: () => setSidebarCollapsed(prev => !prev), | ||
| })); |
There was a problem hiding this comment.
The sidebar and upload-modal state is owned by this NotebookView but mirrored back to the parent via useEffect callbacks below in this file, and the parent talks back via an imperative ref.
This means two copies of same state, you could just lift the sidebarCollapsed and isUploadModalOpen states into the parent and pass them down as props, this will remove the forwardRef, imperative handle and 2 of 3 use effects to sync states.
| useEffect(() => { | ||
| onSidebarCollapsedChange?.(sidebarCollapsed); | ||
| }, [sidebarCollapsed, onSidebarCollapsedChange]); | ||
|
|
||
| useEffect(() => { | ||
| onUploadModalOpenChange?.(isUploadModalOpen); | ||
| }, [isUploadModalOpen, onUploadModalOpenChange]); |
There was a problem hiding this comment.
This ties back to my previous comment on moving the state to the parent to avoid the state mirroring logic.
| position: 'absolute', | ||
| inset: 0, | ||
| margin: 0, | ||
| // padding: 0, |
There was a problem hiding this comment.
| // padding: 0, |
|
@karthikjeeyar We are fixing the modal on top of modal for full screen in this PR #3849 |
…to parent, remove imperative ref - Replace forwardRef/useImperativeHandle with direct props for sidebarCollapsed and isUploadModalOpen - Remove state-sync useEffects, single source of truth now lives in the parent - Remove commented-out code in scoped-dialog-utils - Address karthikjeeyar code review feedback Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
New changes are detected. LGTM label has been removed. |
its-mitesh-kumar
left a comment
its-mitesh-kumar
left a comment
There was a problem hiding this comment.
Review of LightSpeedChat.tsx — a few suggestions to tighten things up.
306ee36 to
f6e63bb
Compare
…tebook-overlay-docked-modes # Conflicts: # workspaces/intelligent-assistant/plugins/intelligent-assistant/src/components/LightSpeedChat.tsx # workspaces/intelligent-assistant/plugins/intelligent-assistant/src/components/notebooks/RenameNotebookModal.tsx
f6e63bb to
d2d693f
Compare
Signed-off-by: rohitratannagar <rohitratannagar2003@gmail.com>
859a4f1 to
92ed5f2
Compare
Signed-off-by: rohitratannagar <rohitratannagar2003@gmail.com>
|
|
Updated header to match prototype: Screen.Recording.2026-08-11.at.11.48.48.PM.mov |
6 participants
Summary
Changes
isCompactprop for full-width sidebar,forwardRef/useImperativeHandlefor header actionsactiveNotebookId/setActiveNotebookIdto contextoverflowX: hiddentooverflow: hiddenon modal to prevent vertical scrollJira
RHIDP-14656
Test plan
yarn tsc,yarn lint,yarn prettier:check,yarn test:all— all pass (2 pre-existing failures inuseLightspeedProviderState.test.tsx)RHIDP-14656
🤖 Generated with Claude Code