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

feat: @llamaindex/chat-ui components #1

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

Conversation

thucpn
Copy link
Collaborator

@thucpn thucpn commented Oct 21, 2024

No description provided.

<div
ref={scrollableChatContainerRef}
className={cn(
'relative flex-1 overflow-y-auto rounded-xl bg-white p-4 shadow-xl',
Copy link
Collaborator

Choose a reason for hiding this comment

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

the components still contain a lot of styling compared to the radix primitives. e.g. here how could a user not use rounded-xl or shadow-xl?

Copy link
Collaborator Author

@thucpn thucpn Oct 23, 2024

Choose a reason for hiding this comment

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

They can custom style of ChatMessages like this:

<ChatMessages className="rounded-none shadow-none" />

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the components still contain a lot of styling compared to the radix primitives

Yes, actually we can remove css like border, shadow, background, color. But I think we should let these default styles like Shadcn do for their components (eg. Shadcn Button has color and rounded corner).

Copy link
Collaborator

Choose a reason for hiding this comment

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

AFAIK shadcn just provides styling templates of style-less radix components. So the shadcn button is styled here https://github.com/shadcn-ui/ui/blob/main/apps/www/registry/default/ui/button.tsx but it uses the unstyled slot here: https://github.com/radix-ui/primitives/blob/main/packages/react/slot/src/Slot.tsx

Copy link
Collaborator

Choose a reason for hiding this comment

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

therefore shadcn code is to be copied (because it contains all the style) and then the user can modify it as wanted

Copy link
Collaborator

@marcusschiesser marcusschiesser Oct 23, 2024

Choose a reason for hiding this comment

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

so, from my understanding, we have two options:

  1. keep it as is: as a result, there will be some default styling in this package
  2. remove styling completely and move the style to the code generated by create-llama

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think we should remove unnecessary styling like: rounded-xl bg-white p-4 shadow-xl and put them in then generated code of create-llama.
The position or layout styling (relative flex ...) is important so we can keep them in this package

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good for me!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@elviskahoro, what are your thoughts on re-using these components in Reflex?

packages/chat-ui/src/chat/chat-input.tsx Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@@ -3,7 +3,7 @@
@tailwind utilities;

@layer base {
:root {
.llamaindex-chat-ui {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we inline this? then the user does not need to import style.css

packages/chat-ui/src/chat/markdown.tsx Outdated Show resolved Hide resolved
packages/chat-ui/package.json Outdated Show resolved Hide resolved
packages/chat-ui/src/adapter/vercel-ai-sdk.ts Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated
import { useChatInput } from '@llamaindex/chat-ui'

const LlamaCloudSelector = () => {
const { data, setData } = useChat()
Copy link
Collaborator

Choose a reason for hiding this comment

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

update with changes of example above

README.md Outdated
Or you can extend them with your own children components and styles:

```tsx
import '@llamaindex/chat-ui/styles.css'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
import '@llamaindex/chat-ui/styles.css'

apps/web/app/chat-section.tsx Outdated Show resolved Hide resolved
@@ -0,0 +1,27 @@
.llamaindex-chat-ui {
Copy link
Collaborator

Choose a reason for hiding this comment

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

aren't the shadcn components just using :root? so we do we need a own namespace here?

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