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

Implement *_reply for the web #81

Merged
merged 1 commit into from
Dec 27, 2022
Merged

Implement *_reply for the web #81

merged 1 commit into from
Dec 27, 2022

Conversation

AbdBarho
Copy link
Collaborator

@AbdBarho AbdBarho commented Dec 27, 2022

Closes #30
Closes #31

I have started to consolidate some of the re-used components to make the code easier to read, I did not change files modified in #76, this could be done in another pr.

I am unsure how "clean" the code should be for the mvp, but we can always clean it up later.

website/src/pages/create/assistant_reply.tsx Outdated Show resolved Hide resolved
website/src/pages/create/user_reply.tsx Outdated Show resolved Hide resolved
website/src/pages/create/user_reply.tsx Outdated Show resolved Hide resolved
Also, start extracting shared components

const getColor = (isAssistant: boolean) => (isAssistant ? "slate-800" : "sky-900");

export const Messages = ({ messages }: { messages: Message[] }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

For a later date, this seems like a viable component for Chakra-ui style style variants. I have an example of this in a test library I made in https://github.com/SurfaceData/surface-react-components/blob/main/src/themes/surface/components/button.ts and https://github.com/SurfaceData/surface-react-components/blob/main/src/Button/Button.tsx.

This would cut down the code and let us drop the getColor method. But we can do that cleanup later.

@fozziethebeat
Copy link
Collaborator

Merging this now as I see changes have been made.

@fozziethebeat fozziethebeat merged commit 38266dd into LAION-AI:main Dec 27, 2022
@AbdBarho AbdBarho deleted the reply branch December 27, 2022 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement assistant_reply for web Implement user_reply for web
2 participants