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

chore: UI upt sync search with alpha branch #223

Draft
wants to merge 5 commits into
base: search
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions apps/masterbots.ai/app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@ import { GeistSans } from 'geist/font/sans'
import { Toaster } from 'react-hot-toast'

import '@/app/globals.css'
import { cookies } from 'next/headers'
import dynamic from 'next/dynamic'
import { Metadata } from 'next/types'
import { objectToCamel } from 'ts-case-convert'
import { Header } from '@/components/layout/header'
import { Providers } from '@/components/layout/providers'
import { cn } from '@/lib/utils'
import { GlobalStoreProvider } from '@/hooks/use-global-store'
import { cn } from '@/lib/utils'
import { createSupabaseServerClient } from '@/services/supabase'
import { GoogleAnalytics } from '@next/third-parties/google'
import { MB } from '@repo/supabase'
import dynamic from 'next/dynamic'
import { cookies } from 'next/headers'
import { Metadata } from 'next/types'
import { objectToCamel } from 'ts-case-convert'

async function getCookieData(): Promise<{ userProfile }> {
const userProfile = cookies().get('userProfile')?.value || null
Expand Down Expand Up @@ -41,7 +43,7 @@ export default async function RootLayout({ children }: RootLayoutProps) {
<Toaster />
<GlobalStoreProvider
categories={categories}
chatbots={[]}
chatbots={chatbots}
user={(userProfile && JSON.parse(userProfile)) || null}
>
<Providers
Expand All @@ -59,6 +61,9 @@ export default async function RootLayout({ children }: RootLayoutProps) {
<DynamicCmdK />
</Providers>
</GlobalStoreProvider>

<GoogleAnalytics gaId="G-N135BF99HS" />

</body>
</html>
)
Expand All @@ -70,7 +75,11 @@ async function getGlobalData() {
const categories = await supabase.from('category').select()
const chatbot = await supabase.from('chatbot').select(`*, prompt(*)`)

return objectToCamel({ categories: categories.data, chatbots: chatbot.data })
return objectToCamel({
categories: categories.data,
// TODO: fix type... it shouldn't be unknown. Moving forward on other places.
chatbots: chatbot.data as unknown as MB.ChatbotWithPrompts[]
})
}

export const viewport = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
'use client'
Copy link
Member

Choose a reason for hiding this comment

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

@AndlerRL dont use client for this. use aria-data css.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why aria on the CSS? The idea here is to scroll into the thread location, not to change the styling. How do I use then client functions without a client component? The scrollIntoView is client side. Why not to use client here? 🤔


import { toSlug } from '@/lib/url-params'
import { MB } from '@repo/supabase'
import { useEffect } from 'react'
import { CategoryLink } from './category-link'

export function CategoryTabs({
Expand All @@ -8,6 +12,26 @@ export function CategoryTabs({
categories: MB.Category[]
initialCategory?: string
}) {
useEffect(() => {
if (document) {
const element = document.getElementById(
`browse-category-tab__${initialCategory === 'all'
? 'all'
: categories.filter(c => toSlug(c.name) === initialCategory)[0]
?.categoryId
}`
)

if (element) {
element.scrollIntoView({
behavior: 'smooth',
block: 'center',
inline: 'center'
})
}
}
}, [initialCategory])

return (
<div className="w-full py-[10px] my-3 !overflow-x-auto overflow-y-hidden whitespace-nowrap scrollbar small-thumb justify-between flex">
<CategoryLink category="all" id="browse-category-tab__null" />
Expand Down
15 changes: 7 additions & 8 deletions apps/masterbots.ai/components/shared/thread-accordion.client.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
'use client'

import { useEffect } from 'react'
import { useQuery } from '@tanstack/react-query'
import { usePathname, useRouter } from 'next/navigation'
import { MB } from '@repo/supabase'
import { getMessagePairs } from '@/app/actions'
import {
Accordion,
AccordionContent,
AccordionItem,
AccordionTrigger
} from '@/components/ui/accordion'
import { getThreadLink } from '@/lib/threads'
import { cn } from '@/lib/utils'
import { getMessagePairs } from '@/app/actions'
import { ThreadHeading } from './thread-heading'
import { MB } from '@repo/supabase'
import { useQuery } from '@tanstack/react-query'
import { usePathname, useRouter } from 'next/navigation'
import { useEffect } from 'react'
import { BrowseChatMessage } from './thread-message'
import { getThreadLink } from '@/lib/threads'

export function ThreadAccordionClient({
thread,
Expand Down Expand Up @@ -56,7 +55,7 @@ export function ThreadAccordionClient({
return (
<div className="flex w-full">
<Accordion
className={cn('w-full border border-solid border-mirage scroll')}
className={cn('w-full border border-solid border-mirage scroll border-r-[transparent]')}
defaultValue={['pair-0', 'pair-1', 'pair-2']}
key={`accordion-${JSON.stringify(pairs)}`}
type="multiple"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { MB } from '@repo/supabase'
import {
Accordion,
AccordionContent,
AccordionItem,
AccordionTrigger
} from '@/components/ui/accordion'
import { cn } from '@/lib/utils'
import { MB } from '@repo/supabase'
import { ThreadHeading } from './thread-heading'
import { BrowseChatMessage } from './thread-message'

Expand All @@ -16,7 +16,7 @@ export function ThreadAccordionServer({
return (
<div className="flex w-full">
<Accordion
className={cn('w-full border border-solid border-mirage scroll')}
className={cn('w-full border border-solid border-mirage scroll border-r-[transparent]')}
defaultValue={['pair-0', 'pair-1', 'pair-2']}
type="multiple"
>
Expand Down
62 changes: 49 additions & 13 deletions apps/masterbots.ai/components/shared/thread-list-accordion.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,62 @@
import { DialogProps } from '@radix-ui/react-dialog'
import type { MB } from '@repo/supabase'
import { cn } from '@/lib/utils'
'use client'

Copy link
Member

Choose a reason for hiding this comment

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

dont use client here, remember we need to discuss as team everytime we want to use client

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any reason to not use use client? Why to put all the weight to the server side? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

  • we are following next14 app router standards to benefits from nextjs features, that's the point.
  • you need to use client as long resort when any other way to do it with as server component.
  • all state needs to be handled thru url and aria data first as in <ThreadHeading> we use data-[state=open]:bg-mirage instead of const[open,setOpen] = useState().
  • when you use client on layout component can possibly get out of sync with nextjs app router state and therefore not update the rendered data.
  • best practice is use client only in small components like buttons and forms, but not on layout components cos you loose a lot of the benefits of using nextjs, basically you turn the whole app into a regular react app without next. you need follow the nextjs standard way.
  • when you start using use client you not only go against the technical design but also starting to create web vitals issues

import {
Accordion,
AccordionContent,
AccordionItem,
AccordionTrigger
} from '@/components/ui/accordion'
import { createMessagePairs } from '@/lib/threads'
import { cn, sleep } from '@/lib/utils'
import { DialogProps } from '@radix-ui/react-dialog'
import type { MB } from '@repo/supabase'
import { useSearchParams } from 'next/navigation'
import { useEffect, useRef } from 'react'
import { useSetState } from 'react-use'
import { ThreadAccordionClient } from './thread-accordion.client'
import { ThreadHeading } from './thread-heading'
import { createMessagePairs } from '@/lib/threads'

export function ThreadListAccordion({
thread,
chat = false
chat = false,
isUser = false,
isBot = false
}: ThreadListAccordionProps) {
const [state, setState] = useSetState({
isOpen: false,
firstQuestion:
thread.messages.find(m => m.role === 'user')?.content || 'not found',
firstResponse:
thread.messages.find(m => m.role === 'assistant')?.content || 'not found'
})
const searchParams = useSearchParams();
const threadRef = useRef<HTMLDivElement>(null)
const handleThreadIdChange = async () => {
if (searchParams.get('threadId') === thread.threadId) {
await sleep(300) // animation time
threadRef.current?.scrollIntoView({ behavior: 'smooth', block: 'start' })
} else if (state.isOpen && searchParams.get('threadId')) {
setState({ isOpen: false })
}
}

useEffect(() => {
handleThreadIdChange()
}, [searchParams])

return (
<Accordion className="w-full" type="multiple">
<AccordionItem value="pair-1">
<Accordion
className="w-full"
onValueChange={v => {
setState({ isOpen: v[0] === 'pair-1' })
}}
type="multiple"
>
{/* Frist level question and excerpt visible on lists */}
<AccordionItem className="border-b-mirage border-solid relative" value="pair-1">
<AccordionTrigger
className={cn('hover:bg-mirage px-5 data-[state=open]:bg-mirage')}
isSticky
className={cn('hover:bg-mirage px-2 border border-[transparent] dark:border-b-mirage border-b-gray-300 rounded-t-lg', state.isOpen && 'bg-mirage')}
>
<ThreadHeading
chat={chat}
Expand All @@ -36,11 +73,8 @@ export function ThreadListAccordion({
<div className="overflow-y-scroll scrollbar srcoll-smooth max-h-[500px]">
<ThreadAccordionClient
chat={chat}
initialMessagePairs={createMessagePairs([
thread.firstMessage,
thread.firstAnswer
])}
thread={thread}
initialMessagePairs={createMessagePairs(thread.messages)}
/>
</div>
</AccordionContent>
Expand All @@ -50,6 +84,8 @@ export function ThreadListAccordion({
}

interface ThreadListAccordionProps extends DialogProps {
thread: MB.ThreadFull
thread: MB.Thread
chat?: boolean
isUser?: boolean
isBot?: boolean
}
11 changes: 8 additions & 3 deletions apps/masterbots.ai/components/shared/thread-list-reload.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
'use client'

import React, { useEffect, useRef, useState } from 'react'
import { useSearchParams, useRouter } from 'next/navigation'

import { useRouter, useSearchParams } from 'next/navigation'
import { useEffect, useRef, useState } from 'react'

// ! This component is not necessary to be split this way, only functionality.
Copy link
Member

Choose a reason for hiding this comment

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

This is work in progress, you can ignore it

// ! Though functionality looks correct, this causes an error in the app, a infinite loop very easy
// ! I suggest to only split the code in the same file, or use a different approach to split the code.
// * Search params might look fine but if an user goes (or saves) a link with a page number, it will be lost the first items that are on smaller page number
// * E.g.: If I go to /?page=2, I will lose the first items that are on page 1... This is not a good user experience
export function ThreadListReload() {
const params = useSearchParams()
const router = useRouter()
Expand Down
Loading
Loading