Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[front] - chore: make InputBar sticky #6275

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

JulesBelveze
Copy link
Contributor

@JulesBelveze JulesBelveze commented Jul 17, 2024

Description

This PR makes the InputBar sticky and addresses various UI layout and overflow issues across multiple pages. It also streamlines scrolling and agent click handling in the assistant conversation component.

Risk

Breaking UI

Deploy Plan

  • Deploy to front-edge
  • Deploy front

@JulesBelveze JulesBelveze force-pushed the chore/make-input-sticky branch 2 times, most recently from 60bbf6e to d4d5462 Compare July 17, 2024 09:55
@JulesBelveze
Copy link
Contributor Author

JulesBelveze commented Jul 17, 2024

Main challenge is that one the layout is slightly broken and a adding sticky top-x to the InputBar doesn't fix. We need to have one div taking the full width & height of the viewport minus the navigation bar. And adding a layout in this div.

The current implementation achieves the intent however the scroll bar is not on the edge of the window (cf screenshot)

Screenshot 2024-07-17 at 22 21 33

See below picture of the unwanted div

Screenshot 2024-07-17 at 22 31 03

EDIT: this has been solved by allowing the outer div to expand to the full width and constraining the inner div.

@@ -492,7 +492,7 @@ function SlackIntegrationDrawer({
hasChanged={false}
variant="side-sm"
>
<div className="pt-8">
<div className="w-full max-w-4xl pt-8">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add pt-8 in here:

isWideMode ? "items-center" : "max-w-4xl px-6"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

☝️ Meant for other places where pt-8 makes sense.

@@ -48,7 +48,7 @@ export function DropzoneContainer({
return (
<div
{...getRootProps()}
className="flex h-full w-full flex-col items-center"
className="flex h-full w-full flex-col items-center overflow-y-auto"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure to understand why we need this one?

@@ -188,8 +185,8 @@ export default function AppLayout({

<div
className={classNames(
"flex h-[calc(100%-5rem)] w-full flex-col",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😘

Jules and others added 9 commits July 30, 2024 15:48
 - Adjust sticky positioning in AssistantInputBar to avoid overlap with other elements
 - Enable scrolling for content in DropzoneContainer to manage overflow properly
 - Ensure the main content container in AppLayout allows for horizontal overflow
 - Add top padding on the Developers tools page for better visual alignment
 - Fix layout inconsistencies on Workspace Assistants page by adding top padding
 - Address overflow issues in managed data sources view for better content accessibility
 - Improve layout on public URLs, static data sources, members, and subscription pages by adding top padding for consistency
 - Removed the automatic scrolling and element visibility check on assistant click
 - Simplified the agent selection on click to directly call the click handler without additional logic

[conversation] - refactor: remove unused IntersectionObserver for input mention

 - The IntersectionObserver related to the assistant input is no longer necessary and has been removed

[misc] - fix: ensure full height utilization in Dropzone container

 - Adjusted styling to allow Dropzone container to use full height for its elements

[pages] - style: add padding to top of new app creation page

 - Introduced top padding to improve the visual layout of the new app creation page
…ehavior

- Apply consistent maximum width to main content containers across various components for a unified look
- Set overflow behavior on assistant list to only clip in the y-direction for better content handling
 - Ensure full height is applied to the subscription plan container on small screens for styling consistency
 - Fixed the bottom border property for a consistent UI in the input bar component
 - Improved positioning for sticky input bar on larger screens with responsive top alignment
 - Removed unnecessary wrappers to simplify the React component trees
 - Updated class names to improve CSS consistency across components
 - Adjusted page layouts for better content alignment and spacing
 - Updated button handling for Slack integration and data-source connection to improve UX
 - Fixed inconsistent use of design tokens for font sizes and element spacing
 - Enhanced responsiveness of components to work better on various screen sizes
 - Streamlined the elements in assistants and developers tools pages for better readability
 - Remove `h-full` css class to maintain consistent container size across components
 - Standardize `overflow-y-auto` to improve scroll behavior and layout structure
- Prevent horizontal overflow in the main content's flex wrapper div to ensure a cleaner layout
- Allow vertical scrolling within the main content area to facilitate content navigation when content exceeds the viewport height
…ayout mode

- Add center alignment for content in wide mode, ensuring it remains visually balanced on the page

[front/pages/w] - refactor: remove overflow-y-clip for better content flow

- Remove unnecessary overflow clip for workspace assistants, allowing for a smoother content expansion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants