Skip to content
This repository was archived by the owner on Mar 7, 2026. It is now read-only.

Update Chat UI#367

Merged
joseplayero merged 32 commits intoreorproject:mainfrom
milaiwi:update-chat-ui
Aug 24, 2024
Merged

Update Chat UI#367
joseplayero merged 32 commits intoreorproject:mainfrom
milaiwi:update-chat-ui

Conversation

@milaiwi
Copy link
Copy Markdown
Collaborator

@milaiwi milaiwi commented Aug 21, 2024

closes #341
/claim #341

Lots of changes. I've tested as much as I can on my side, but let me know if you run into any issues. Some things to look out for:

  1. Changing tabs/chats after submitting should stop streamingResponse
  2. Clicking a prompt suggestion should go to new chat, display loadAnimation, then hide when response comes in
  3. Checking overflow on TextArea
  4. Changing size of windows

I believe I covered all of this, but since I made lots of changes I might have missed something.

Changes

  • Added loadAnimation to display when ChatInput submits. Clears when we get first response from streamingResponse
  • loadAnimation and loadResponse resets when changing tabs
  • Basic Animations
  • Changed TextArea to textarea on ChatInput since it's more dynamic when it comes to CSS
  • Fixed bug where changing chat while streamingResponse caused it to append on the newly selected currentChatHistory.displayableChatHistory

Things to look for

  • I did not merge with the previous PR on opening chat appearing on TabBar since I ran into bugs of it creating files in my FileSidebar that should not exist, hovering over the tab caused an issue with the Tooltip, and phantom tabs.

Future Addition

  • It would be nice if the API allowed for a function to call that cleans up streamingResponse when we no longer need the content. Right now, there is no way to stop it from streamingResponse so if we change tabs we have to wait for our content from finish appending. I can make a post on this if you want.

@algora-pbc algora-pbc bot mentioned this pull request Aug 21, 2024
@algora-pbc
Copy link
Copy Markdown

algora-pbc bot commented Aug 21, 2024

💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe.

Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR significantly updates the Chat UI, addressing the issues mentioned in #341 and introducing new features to enhance user experience.

  • Improved contrast and padding in chat messages (src/styles/chat.css)
  • Redesigned ChatInput component with dynamic height adjustment (src/components/Chat/ChatInput.tsx)
  • Added loading animations for better user feedback (src/utils/animations.tsx)
  • Implemented collapsible 'Recents' section in ChatsSidebar (src/components/Chat/ChatsSidebar.tsx)
  • Introduced new color variables and custom font in tailwind.config.js for consistent styling

10 file(s) reviewed, 17 comment(s)
Edit PR Review Bot Settings

text-base
text-white shadow hover:bg-neutral-300 hover:text-black`}
className={`mx-1 mt-2
h-[100px] max-w-[200px] cursor-pointer rounded-lg border
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: Consider using Tailwind's aspect-square class instead of fixed height for better responsiveness

[setCurrentChatHistory], // Add any other dependencies here if needed
)

/* eslint-disable */
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: This function is marked with eslint-disable. Consider addressing the linting issues instead of disabling eslint.

Comment on lines +355 to +368
/* eslint-disable */
useEffect(() => {
// Update context when the chat history changes
const context = getChatHistoryContext(currentChatHistory)
setCurrentContext(context)

if (!promptSelected) {
setLoadAnimation(false)
setLoadingResponse(false)
} else {
setPromptSelected(false)
}
}, [currentChatHistory?.id])
/* eslint-enable */
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: This useEffect hook is also marked with eslint-disable. Consider addressing the linting issues.

if (promptSelected) {
handleSubmitNewMessage(undefined)
}
/* eslint-disable-next-line react-hooks/exhaustive-deps */
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: This eslint-disable-next-line comment suggests there might be a dependency array issue. Review the dependencies for this useEffect hook.

Comment on lines +403 to +404
/* eslint-disable-next-line react-hooks/exhaustive-deps */
}, [appendNewContentToMessageHistory, loadingResponse, currentChatHistory?.id])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: Another eslint-disable-next-line comment. Consider addressing the dependency array issue for this useEffect hook.

}

.chat-container p {
font-family: ui-sans-serif, -apple-system, system-ui, "Segoe UI", Roboto, Ubuntu, Cantarell, "Noto Sans", sans-serif, "Helvetica", "Apple Color Emoji", Arial, "Segoe UI Emoji", "Segoe UI Symbol";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: long font-family declaration may benefit from being defined as a CSS variable for reusability

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is very similar to tailwind builtin font-sans

Comment on lines +18 to +19
background-color: rgb(50, 50, 50);
color: rgb(220, 220, 220);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: consider using CSS variables for colors to maintain consistency and ease future updates

Comment on lines +28 to +29
.assistant-chat-message {
color: rgb(210, 210, 210);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: assistant message lacks background color, which may affect readability on different themes

Comment on lines +36 to +45
@keyframes dropdown-fadeIn {
from {
opacity: 0;
transform: translateY(0);
}
to {
opacity: 1;
transform: translateY(10px);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: animation transforms from 0 to 10px, which may be too subtle. Consider increasing the distance for a more noticeable effect

Comment on lines +6 to +8
<span className="animate-bounce">.</span>
<span className="animate-bounce delay-200">.</span>
<span className="animate-bounce delay-[400ms]">.</span>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: Extract delay values to constants for easier maintenance

<img src="/src/assets/reor-logo.svg" style={{ width: '64px', height: '64px' }} alt="ReorImage" />
</div>
<h1 className="mb-10 text-[28px] text-gray-300">
Welcome to your AI-powered assistant! Start your first conversation or pick up where you left off.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the second sentence after exclamation should be:

Start a conversation with your second brain...

const [userTextFieldInput, setUserTextFieldInput] = useState<string>('')
const [askText] = useState<AskOptions>(AskOptions.Ask)
const [loadingResponse, setLoadingResponse] = useState<boolean>(false)
const [loadAnimation, setLoadAnimation] = useState<boolean>(false)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it seems like we use loadingResponse and loadAnimation as basically the same thing - could you remove one of them please?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

They're not the same thing. loadAnimation is the state that triggers the LoadingDots when we are waiting for a response from streamingResponse. If you use a remote API or have a slow machine, then the latency is very apparent and there needs to exist some way to know that everything is working properly and the LLM is going to send a response. loadAnimation gets set to false on the very first receive from streamingResponse

loadingResponse continues until we are completely done from getting a response from the LLM. That is to say that loadAnimation stops on the first data received, while loadingResponse stops at the very end. It's purpose is to prevent the user from submitting another message or input while we are still streaming.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ah gotcha. In that case to make the code easy to understand perhaps using a union type for all the states in loading response would be nice?

Copy link
Copy Markdown
Collaborator

@joseplayero joseplayero left a comment

Choose a reason for hiding this comment

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

thanks for getting this together -Some queries:

  • Noting here the feature that was recently added for copying chat output to clipboard and creating file from chat output has been lost
  • Could you merge main into this so that we can properly test the changes
  • The prompt suggestions should be more semantic-retrieval prompts like "What have I written about philosophy", "which politicians have i discussed positively" rather than the ones you've added which would not retrieve anything useful from the users notes

},
fontFamily: {
// "material-icons": ["Material Icons"],
'styrene': [
Copy link
Copy Markdown
Collaborator

@joseplayero joseplayero Aug 23, 2024

Choose a reason for hiding this comment

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

do we use this font?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it seems like we define a very similar set of prompts above in the chat-container p css

Copy link
Copy Markdown
Collaborator

@joseplayero joseplayero left a comment

Choose a reason for hiding this comment

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

looks great @milaiwi

Tiny point: I think we need a "Send" or "Ask" button in new chat - but this can come alongside the work to improve the context settings for chat

I've also added a comment on the css/tailwind

@joseplayero
Copy link
Copy Markdown
Collaborator

/tip $30 @milaiwi

@algora-pbc
Copy link
Copy Markdown

algora-pbc bot commented Aug 23, 2024

@algora-pbc
Copy link
Copy Markdown

algora-pbc bot commented Aug 23, 2024

@milaiwi: You just got a $30 tip! 👉 Complete your Algora onboarding to collect your payment.

@algora-pbc
Copy link
Copy Markdown

algora-pbc bot commented Aug 24, 2024

🎉🎈 @milaiwi has been awarded $30! 🎈🎊

@milaiwi
Copy link
Copy Markdown
Collaborator Author

milaiwi commented Aug 24, 2024

I'll fix this up and make the last commit

@joseplayero joseplayero merged commit 51aecbc into reorproject:main Aug 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Redesign Chat UI

2 participants