chore: UI upt sync search with alpha branch #223
Conversation
* feat: setup google analytics tag * wip: rm comment
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Important Review SkippedAuto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
| @@ -1,4 +1,8 @@ | |||
| 'use client' | |||
There was a problem hiding this comment.
@AndlerRL dont use client for this. use aria-data css.
There was a problem hiding this comment.
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 { DialogProps } from '@radix-ui/react-dialog' | ||
| import type { MB } from '@repo/supabase' | ||
| import { cn } from '@/lib/utils' | ||
| 'use client' |
There was a problem hiding this comment.
dont use client here, remember we need to discuss as team everytime we want to use client
There was a problem hiding this comment.
Is there any reason to not use use client? Why to put all the weight to the server side? 🤔
There was a problem hiding this comment.
- we are following next14 app router standards to benefits from nextjs features, that's the point.
- you need to
use clientas 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 usedata-[state=open]:bg-mirageinstead ofconst[open,setOpen] = useState(). - when you
use clienton layout component can possibly get out of sync with nextjs app router state and therefore not update the rendered data. - best practice is
use clientonly 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 clientyou not only go against the technical design but also starting to create web vitals issues
| import { useRouter, useSearchParams } from 'next/navigation' | ||
| import { useEffect, useRef, useState } from 'react' | ||
|
|
||
| // ! This component is not necessary to be split this way, only functionality. |
There was a problem hiding this comment.
This is work in progress, you can ignore it
|
|
||
| const queryKey = [usePathname(), query] | ||
| const [lastQueryKey, setLastQueryKey] = useState(queryKey) | ||
| const { isFetchingNextPage, fetchNextPage, data, refetch } = useInfiniteQuery( |
There was a problem hiding this comment.
we dont want use client. that's why we are doing infinite scrolling the nextjs way
|
Pretty much all changes are not needed, all we need to add to |
|
As mentioned at this comment, these updates are not longer required: @TopETH was requested to check the missing fixes that are here to develop AND prior that to the |
Changelog:
TODO:
searchbranch.