[front] - feat: toggle sidebar #5211

JulesBelveze · 2024-05-22T12:25:43Z

Description

This PR aims at enabling toggle/untoggle the navigation bar on laptop: #4988 (comment)

Screenshots of how it will look like:

Risk

Breaking UI

Deploy Plan

…igation - Added context state management for handling the visibility of the laptop navigation bar - Implemented a button to allow users to toggle the laptop navigation bar on and off - Wrapped the laptop navigation bar inside a Transition.Root component to control its visibility based on state - Modified the SidebarProvider to include the new state and context for laptop navigation control

- Extract `laptopNavOpen` state management into a new `NavigationBarProvider` - Wrap `RootLayout` with the `NavigationBarProvider` for handling navigation bar state [front/components/sparkle] - refactor: remove laptop navigation state from SidebarContext - Remove `laptopNavOpen` and its state management functions from `SidebarContext` - Adjust `SidebarProvider` to only provide `sidebarOpen` state after the split of contexts

- Removed redundant IconButton and Button imports not in use - Introduced ToggleSideBarButton component for cleaner code and separation of concerns - Removed inline css and redundant span elements for toggle button - Adjusted main content conditional padding logic for consistency - Cleaned up unnecessary class conditional logic simplifying readability - Ensured consistent use of classNames helper function

- Conditionally apply sidebar padding in AppLayout only when laptopNavOpen flag is true, preserving layout on smaller screens

- Upgraded the @dust-tt/sparkle package including new features and bug fixes - Added dependency on lottie-web version 5.12.2 to the sparkle package

…n `AppLayout.tsx` - Replace a custom toggle button implementation with a `CollapseButton` component from the sparkle library - Use `useRef` and `useCallback` for button reference and click event handler to optimize component performance - Maintain the toggle state for left and right direction using a local state variable

- Change the input bar from fixed to absolute positioning to improve layout management - Ensure input bar stays at the bottom, now wrapped in an additional div with relative positioning for better control on placement

JulesBelveze

Note

Introduced a NavigationBarContext to mimic the SidebarContext. Curious to hear if this is no good practice?

Question

Why do we render a different nav bar for laptop and mobile? Why not a single instance deciding where to locate based on the device?

JulesBelveze · 2024-05-22T14:31:38Z

front/components/assistant/conversation/input_bar/InputBar.tsx

+    <div className="relative z-20 flex-initial">
+      <div className="4xl:px-0 absolute bottom-0 left-0 right-0">
+        <div className="mx-auto max-h-screen max-w-4xl pb-0 sm:pb-8">
+          <AssistantInputBar
+            owner={owner}
+            onSubmit={onSubmit}
+            conversationId={conversationId}
+            stickyMentions={stickyMentions}
+            additionalAgentConfiguration={additionalAgentConfiguration}
+            hideQuickActions={hideQuickActions}
+            disableAutoFocus={disableAutoFocus}
+          />
+        </div>


Had to modify the classes as fixed was disabling the AssistantInputBar to me translated

roger that will test 👍

spolu

Left a few comments. Will test locally.

Deferring review to @flvndvd as he has state/plans/ideas around the AppLayout

front/components/sparkle/AppLayout.tsx

spolu · 2024-05-22T15:29:44Z

front/components/assistant/conversation/input_bar/InputBar.tsx

+    <div className="relative z-20 flex-initial">
+      <div className="4xl:px-0 absolute bottom-0 left-0 right-0">
+        <div className="mx-auto max-h-screen max-w-4xl pb-0 sm:pb-8">
+          <AssistantInputBar
+            owner={owner}
+            onSubmit={onSubmit}
+            conversationId={conversationId}
+            stickyMentions={stickyMentions}
+            additionalAgentConfiguration={additionalAgentConfiguration}
+            hideQuickActions={hideQuickActions}
+            disableAutoFocus={disableAutoFocus}
+          />
+        </div>


roger that will test 👍

front/components/sparkle/AppLayout.tsx

- Renamed `laptopNavOpen` and `setLaptopNavOpen` to `navigationBarOpen` and `setNavigationBarOpen` for improved code readability - Updated associated usage and context of navigation bar state variables across the `AppLayout` component

…ng of navigation bar - Simplified the logic for displaying the navigation bar by removing the Transition.Root component when hideSidebar is false and navigationBarOpen is true

…RootLayout - Extracted NavigationBarProvider dependency from the RootLayout component [front/components/sparkle] - refactor: remove NavigationBarContext and Provider - Deleted NavigationBarContext and NavigationBarProvider to simplify the context providers - Moved navigationBar state management directly into AppLayout using useState

front/components/sparkle/AppLayout.tsx

- Removed unnecessary padding in wide mode to utilize full screen width - Added consistent padding for the assistant new page to align with design standards

- Renamed prop from `setNavigationBarOpen` to `toggleNavigationBarVisibility` to improve function name semantics - Simplified div structures in AppLayout by removing unnecessary className conditions - Updated corresponding functions to use the new toggleNavigationBarVisibility prop for clarity and consistency

- Removed the `isWideMode` prop from `AppLayout` usage in `ConversationLayout.tsx` and `AssistantBuilder.tsx` files to streamline layout options - Adjusted styling in `AppLayout.tsx` by consolidating the `children` rendering into a single line for cleanliness and readability - Standardized padding styles in `assistant/new.tsx` for better code consistency

flvndvd · 2024-05-23T14:38:22Z

The width of the messages has changed:

flvndvd · 2024-05-23T14:09:53Z

front/components/sparkle/AppLayout.tsx

+      className={`hidden lg:fixed lg:top-1/2 lg:flex lg:h-10 lg:w-5 ${
+        navigationBarOpen ? "lg:left-80" : "lg:left-2"
+      }`}


You can use the helper classNames().

JulesBelveze · 2024-05-24T08:49:12Z

As per IRL discussion with @flvndvd, waiting for him to ship the new discussion design to rebase and use it.

spolu · 2024-05-25T19:15:15Z

What's the new discussion design?

JulesBelveze · 2024-05-26T09:05:16Z

With the rounded text boxes. Just tagged you in the corresponding Figma

spolu · 2024-05-26T09:26:37Z

Ah this PR! 👍 Roger 👍

JulesBelveze · 2024-05-28T19:08:29Z

@flvndvd The only way I've found to fix the centering issue of the FixedAssistantInputBar is to depend on the state of the navigationBar and has something like:

<div className={classNames("4xl:px-0 fixed bottom-0 left-0 right-0 z-20 flex-initial",
      isNavigationBarOpen? "lg:left-80": "")}>
      <div className="mx-auto max-h-screen max-w-4xl pb-0 sm:pb-8">
....

This seems to work but will require to create a NavigationBar context. What do you think?

- Correct the conditional application of positional styling for the navigation bar to respect 'navigationBarOpen' state - Ensure the sidebar does not overlap with navigation bar when it's not supposed to be open

JulesBelveze · 2024-05-29T07:26:00Z

As per IRL we will keep the current implementation, there's a bit of a glitch for viewports with a width in [1024, 1200]px

Opened an issue #5314.

front/components/sparkle/AppLayout.tsx

flvndvd · 2024-05-29T08:09:06Z

front/components/sparkle/AppLayout.tsx

+      className={`hidden lg:fixed lg:top-1/2 lg:flex lg:h-10 lg:w-5 ${
+        navigationBarOpen ? "lg:left-80" : "lg:left-2"
+      }`}


front/components/sparkle/AppLayout.tsx

front/package.json

albandum · 2024-05-29T08:18:14Z

How do you open again the navbar from the first screenshot ?

JulesBelveze · 2024-05-29T08:19:00Z

@albandum There's a small button on the left hand side

…yling - Change parameter name to enhance readability in ToggleSideBarButton component - Adjust left positioning of toggle button when the navigation bar is open - Update state variable for navigation bar visibility to improve clarity throughout AppLayout component - Remove unnecessary top margin and increase button width for better alignment - Update package version for @dust-tt/sparkle to 0.2.153

albandum · 2024-05-29T08:23:51Z

@albandum There's a small button on the left hand side

Ok - not visible from the screenshot (I guess I thought it was something on my screen haha) but if there's some sort of animation that works 👍

flvndvd

You need to remove the changes on pacakge-lock.json as well.

front/components/sparkle/AppLayout.tsx

- Renamed `navigationBarOpen` to `isNavigationBarOpened` for consistency with naming conventions - Used the `classNames` utility for conditionally applying classes to improve readability and maintainability

This reverts commit 1ce657c.

…5373) * Revert "[front] - feature: add option to hide sidebar toggle button (#5360)" This reverts commit 16c0f99. * Revert "[front] - feat: toggle sidebar (#5211)" This reverts commit 1ce657c.

Jules added 4 commits May 22, 2024 11:09

[front] - fix: adjust sidebar padding based on laptopNavOpen state

e24887b

- Conditionally apply sidebar padding in AppLayout only when laptopNavOpen flag is true, preserving layout on smaller screens

JulesBelveze self-assigned this May 22, 2024

Jules added 4 commits May 22, 2024 14:57

[front] - feature: update @dust-tt/sparkle to version 0.2.152

628fd7e

- Upgraded the @dust-tt/sparkle package including new features and bug fixes - Added dependency on lottie-web version 5.12.2 to the sparkle package

Merge branch 'main' into feat/toggle-sidebar

6ddffbd

[front] - refactor: adjust positioning of assistant input bar

9d37365

- Change the input bar from fixed to absolute positioning to improve layout management - Ensure input bar stays at the bottom, now wrapped in an additional div with relative positioning for better control on placement

JulesBelveze commented May 22, 2024

View reviewed changes

JulesBelveze requested a review from spolu May 22, 2024 15:15

spolu reviewed May 22, 2024

View reviewed changes

spolu requested a review from flvndvd May 22, 2024 15:31

spolu reviewed May 22, 2024

View reviewed changes

front/components/sparkle/AppLayout.tsx Outdated Show resolved Hide resolved

Jules added 3 commits May 23, 2024 08:04

[front/components/sparkle] - refactor: streamline conditional renderi…

8ce15dc

…ng of navigation bar - Simplified the logic for displaying the navigation bar by removing the Transition.Root component when hideSidebar is false and navigationBarOpen is true

flvndvd reviewed May 23, 2024

View reviewed changes

front/components/sparkle/AppLayout.tsx Outdated Show resolved Hide resolved

front/components/sparkle/AppLayout.tsx Outdated Show resolved Hide resolved

flvndvd and others added 4 commits May 23, 2024 10:48

Fix new conversation input bar layout

c519280

[front] - fix: adjust layout and padding for consistency

ab7a7b8

- Removed unnecessary padding in wide mode to utilize full screen width - Added consistent padding for the assistant new page to align with design standards

JulesBelveze requested a review from flvndvd May 23, 2024 11:57

flvndvd reviewed May 23, 2024

View reviewed changes

merge main branch

d791626

JulesBelveze assigned flvndvd May 27, 2024

[front] - fix: adjust navigation bar positioning logic

5ae2e29

- Correct the conditional application of positional styling for the navigation bar to respect 'navigationBarOpen' state - Ensure the sidebar does not overlap with navigation bar when it's not supposed to be open

JulesBelveze requested a review from flvndvd May 29, 2024 07:26

flvndvd reviewed May 29, 2024

View reviewed changes

JulesBelveze requested a review from spolu May 29, 2024 08:30

flvndvd reviewed May 29, 2024

View reviewed changes

front/components/sparkle/AppLayout.tsx Outdated Show resolved Hide resolved

front/components/sparkle/AppLayout.tsx Outdated Show resolved Hide resolved

Jules added 2 commits May 29, 2024 10:51

[front] - refactor: standardize prop naming in ToggleSideBarButton

9480537

- Renamed `navigationBarOpen` to `isNavigationBarOpened` for consistency with naming conventions - Used the `classNames` utility for conditionally applying classes to improve readability and maintainability

fix: package-lock.json

970d2fc

JulesBelveze force-pushed the feat/toggle-sidebar branch from 3ccae10 to 970d2fc Compare May 29, 2024 08:55

JulesBelveze requested a review from flvndvd May 29, 2024 09:32

flvndvd approved these changes May 29, 2024

View reviewed changes

JulesBelveze merged commit 1ce657c into main May 29, 2024
3 checks passed

JulesBelveze deleted the feat/toggle-sidebar branch May 29, 2024 16:12

flvndvd added a commit that referenced this pull request May 30, 2024

Revert "[front] - feat: toggle sidebar (#5211)"

c2216a2

This reverts commit 1ce657c.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[front] - feat: toggle sidebar #5211

[front] - feat: toggle sidebar #5211

JulesBelveze commented May 22, 2024 •

edited

Loading

JulesBelveze left a comment •

edited

Loading

JulesBelveze May 22, 2024

spolu May 22, 2024

spolu left a comment

spolu May 22, 2024

flvndvd commented May 23, 2024 •

edited

Loading

flvndvd May 23, 2024

flvndvd May 29, 2024

JulesBelveze commented May 24, 2024

spolu commented May 25, 2024

JulesBelveze commented May 26, 2024

spolu commented May 26, 2024

JulesBelveze commented May 28, 2024

JulesBelveze commented May 29, 2024 •

edited

Loading

flvndvd May 29, 2024

albandum commented May 29, 2024

JulesBelveze commented May 29, 2024

albandum commented May 29, 2024

flvndvd left a comment

[front] - feat: toggle sidebar #5211

[front] - feat: toggle sidebar #5211

Conversation

JulesBelveze commented May 22, 2024 • edited Loading

Description

Risk

Deploy Plan

JulesBelveze left a comment • edited Loading

Choose a reason for hiding this comment

Note

Question

JulesBelveze May 22, 2024

Choose a reason for hiding this comment

spolu May 22, 2024

Choose a reason for hiding this comment

spolu left a comment

Choose a reason for hiding this comment

spolu May 22, 2024

Choose a reason for hiding this comment

flvndvd commented May 23, 2024 • edited Loading

flvndvd May 23, 2024

Choose a reason for hiding this comment

flvndvd May 29, 2024

Choose a reason for hiding this comment

JulesBelveze commented May 24, 2024

spolu commented May 25, 2024

JulesBelveze commented May 26, 2024

spolu commented May 26, 2024

JulesBelveze commented May 28, 2024

JulesBelveze commented May 29, 2024 • edited Loading

flvndvd May 29, 2024

Choose a reason for hiding this comment

albandum commented May 29, 2024

JulesBelveze commented May 29, 2024

albandum commented May 29, 2024

flvndvd left a comment

Choose a reason for hiding this comment

JulesBelveze commented May 22, 2024 •

edited

Loading

JulesBelveze left a comment •

edited

Loading

flvndvd commented May 23, 2024 •

edited

Loading

JulesBelveze commented May 29, 2024 •

edited

Loading