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

fix: resolve React warnings and accessibility issues #1763

Merged
merged 39 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
2f0156e
fix: prevent state updates during render in App component
hexart Jan 21, 2025
a00ed5a
fix: resolve invalid DOM nesting of h2 in p tags in Markdown.tsx
hexart Jan 21, 2025
1c70624
fix: replace p tags with div in message content
hexart Jan 21, 2025
7af4ca8
fix: add key to fragment in Messages component
hexart Jan 21, 2025
3fea378
fix: remove nested button in thread options
hexart Jan 21, 2025
af44de3
fix: resolve button nesting in thread list
hexart Jan 21, 2025
6643e10
fix: handle undefined checks in ChatProfiles
hexart Jan 21, 2025
8f1ae0c
test: update command test to handle whitespace
hexart Jan 21, 2025
15d479a
fix: resolve lint errors
hexart Jan 21, 2025
018a912
fix: add specific eslint disable directives for table components
hexart Jan 21, 2025
4941faa
Merge branch 'main' into fix/app-render-warning
hexart Jan 21, 2025
4936a0c
Merge branch 'main' into fix/app-render-warning
dokterbob Jan 21, 2025
4987bf5
Merge branch 'main' into fix/app-render-warning
hexart Jan 21, 2025
676a2dd
Merge branch 'main' into fix/app-render-warning
hexart Jan 22, 2025
15a249c
Merge branch 'main' into fix/app-render-warning
dokterbob Jan 22, 2025
e9af9aa
Merge branch 'main' into fix/app-render-warning
dokterbob Jan 22, 2025
b25ffec
Merge branch 'main' into fix/app-render-warning
hexart Jan 22, 2025
30b201c
Merge branch 'Chainlit:main' into fix/app-render-warning
hexart Jan 23, 2025
f832467
Merge branch 'main' into fix/app-render-warning
hexart Jan 24, 2025
cd04feb
Merge branch 'Chainlit:main' into fix/app-render-warning
hexart Jan 26, 2025
ab064d1
Merge branch 'main' into fix/app-render-warning
hexart Jan 28, 2025
be7645b
Merge branch 'main' into fix/app-render-warning
hexart Jan 28, 2025
4190688
Merge branch 'main' into fix/app-render-warning
hexart Jan 29, 2025
7831dac
Merge branch 'Chainlit:main' into fix/app-render-warning
hexart Jan 29, 2025
a45c17b
remove some comments
hexart Jan 29, 2025
e6f0b21
fix(markdown): update p component to handle MarkdownAlert nesting
hexart Jan 29, 2025
3b4e331
fix(markdown): update Markdown component
hexart Jan 29, 2025
102c99c
fix: resolve Dialog accessibility error
hexart Jan 30, 2025
43f6858
Update ThreadList
hexart Jan 30, 2025
a75473d
fix: avoid button nesting warning in ThreadOptions component
hexart Jan 30, 2025
08e4a41
Merge branch 'main' into fix/app-render-warning
hexart Jan 30, 2025
fd9f8c9
Merge branch 'Chainlit:main' into fix/app-render-warning
hexart Jan 30, 2025
833275c
refactor(ChatProfiles): optimize profile selection logic
hexart Jan 31, 2025
4f3e7ba
Merge branch 'main' into fix/app-render-warning
hexart Feb 2, 2025
92466ae
perf(markdown): optimize block element detection with useMemo
hexart Feb 4, 2025
59ca18a
Merge branch 'main' into fix/app-render-warning
hexart Feb 4, 2025
87f89ed
fix(markdown): simplify paragraph handling to resolve DOM nesting war…
hexart Feb 5, 2025
d2c2821
fix(markdown): remove unused import and add role='paragraph'
hexart Feb 5, 2025
9d60b96
fix(markdown): changing role='article'
hexart Feb 5, 2025
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
4 changes: 3 additions & 1 deletion cypress/e2e/command/spec.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ describe('Command', () => {

cy.get("#command-button").should("exist")

cy.get(".step").eq(1).should("have.text", "Command: Search")
cy.get(".step").eq(1).invoke('text').then((text) => {
expect(text.trim()).to.equal("Command: Search")
})

cy.get(`#chat-input`).type("/pic")
cy.get(".command-item").should('have.length', 1);
Expand Down
24 changes: 13 additions & 11 deletions frontend/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,29 +38,31 @@ function App() {
: false;

useEffect(() => {
if (!isAuthenticated || !isReady) {
if (!isAuthenticated || !isReady || !chatProfileOk) {
return;
} else if (!chatProfileOk) {
return;
} else {
connect({
transports: window.transports,
userEnv
});
}

connect({
transports: window.transports,
userEnv
});
}, [userEnv, isAuthenticated, connect, isReady, chatProfileOk]);

if (configLoaded && config.chatProfiles.length && !chatProfile) {
// Autoselect the first default chat profile
useEffect(() => {
if (!configLoaded || !config || !config.chatProfiles?.length || chatProfile) {
return;
}

const defaultChatProfile = config.chatProfiles.find(
(profile) => profile.default
);

if (defaultChatProfile) {
setChatProfile(defaultChatProfile.name);
} else {
setChatProfile(config.chatProfiles[0].name);
}
}
}, [configLoaded, config, chatProfile, setChatProfile]);

if (!configLoaded && isAuthenticated) return null;

Expand Down
50 changes: 25 additions & 25 deletions frontend/src/components/LeftSidebar/ThreadList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -274,32 +274,32 @@ export function ThreadList({
isResumed || threadHistory.currentThreadId === thread.id;
return (
<SidebarMenuItem key={thread.id} id={`thread-${thread.id}`}>
<Link to={isResumed ? '' : `/thread/${thread.id}`}>
<SidebarMenuButton
isActive={isSelected}
className="relative truncate h-9 group/thread"
>
{thread.name || (
<Translator path="threadHistory.thread.untitled" />
)}
<div
className={cn(
'absolute w-10 bottom-0 top-0 right-0 bg-gradient-to-l from-[hsl(var(--sidebar-background))] to-transparent'
)}
/>
<ThreadOptions
onDelete={() => setThreadIdToDelete(thread.id)}
onRename={() => {
setThreadIdToRename(thread.id);
setThreadNewName(thread.name);
}}
className={cn(
'absolute z-20 bottom-0 top-0 right-0 bg-sidebar-accent hover:bg-sidebar-accent hover:text-primary flex opacity-0 group-hover/thread:opacity-100',
isSelected && 'bg-sidebar-accent opacity-100'
<div className="relative">
<Link to={isResumed ? '' : `/thread/${thread.id}`}>
<SidebarMenuButton
isActive={isSelected}
className="relative truncate h-9 group/thread pr-10" // 添加右边距给 ThreadOptions 留空间
hexart marked this conversation as resolved.
Show resolved Hide resolved
>
{thread.name || (
<Translator path="threadHistory.thread.untitled" />
)}
/>
</SidebarMenuButton>
</Link>
<div className={cn(
'absolute w-10 bottom-0 top-0 right-0 bg-gradient-to-l from-[hsl(var(--sidebar-background))] to-transparent'
)} />
</SidebarMenuButton>
</Link>
<ThreadOptions
onDelete={() => setThreadIdToDelete(thread.id)}
onRename={() => {
setThreadIdToRename(thread.id);
setThreadNewName(thread.name);
}}
className={cn(
'absolute z-20 right-0 top-0 h-full bg-sidebar-accent hover:bg-sidebar-accent hover:text-primary flex opacity-0 group-hover/thread:opacity-100',
isSelected && 'bg-sidebar-accent opacity-100'
)}
/>
</div>
</SidebarMenuItem>
);
})}
Expand Down
30 changes: 13 additions & 17 deletions frontend/src/components/LeftSidebar/ThreadOptions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { cn } from '@/lib/utils';
import { Ellipsis, Trash } from 'lucide-react';

import { Pencil } from '@/components/icons/Pencil';
import { Button } from '@/components/ui/button';
import { buttonVariants } from '@/components/ui/button';
import {
DropdownMenu,
DropdownMenuContent,
Expand All @@ -25,22 +25,18 @@ export default function ThreadOptions({
}: Props) {
return (
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
variant="ghost"
size="icon"
onClick={(e) => {
e.stopPropagation();
e.preventDefault();
}}
id="thread-options"
className={cn(
'focus:ring-0 focus:ring-offset-0 focus-visible:ring-0 focus-visible:ring-offset-0 text-muted-foreground',
className
)}
>
<Ellipsis />
</Button>
<DropdownMenuTrigger className={cn(
buttonVariants({ variant: 'ghost', size: 'icon' }),
'focus:ring-0 focus:ring-offset-0 focus-visible:ring-0 focus-visible:ring-offset-0 text-muted-foreground',
className
)}
onClick={(e) => {
e.stopPropagation();
e.preventDefault();
}}
id="thread-options"
>
<Ellipsis />
</DropdownMenuTrigger>
<DropdownMenuContent className="w-20" align="start" forceMount>
<DropdownMenuItem
Expand Down
26 changes: 14 additions & 12 deletions frontend/src/components/Markdown.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { cn } from '@/lib/utils';
import { omit } from 'lodash';
import { useContext, useMemo } from 'react';
import React, { useContext, useMemo } from 'react';
import ReactMarkdown from 'react-markdown';
import { PluggableList } from 'react-markdown/lib';
import rehypeKatex from 'rehype-katex';
Expand All @@ -12,9 +12,11 @@ import { visit } from 'unist-util-visit';
import { ChainlitContext, type IMessageElement } from '@chainlit/react-client';

import { AspectRatio } from '@/components/ui/aspect-ratio';
/* eslint-disable-next-line @typescript-eslint/no-unused-vars */
hexart marked this conversation as resolved.
Show resolved Hide resolved
import { Card } from '@/components/ui/card';
import { Separator } from '@/components/ui/separator';
import {
/* eslint-disable-next-line @typescript-eslint/no-unused-vars */
hexart marked this conversation as resolved.
Show resolved Hide resolved
Table,
TableBody,
TableCell,
Expand Down Expand Up @@ -230,18 +232,18 @@ const Markdown = ({
);
},
p(props) {
return (
<p
{...omit(props, ['node'])}
className="leading-7 [&:not(:first-child)]:mt-4 whitespace-pre-wrap break-words"
/>
const containsBlockElement = React.Children.toArray(props.children).some(
child =>
React.isValidElement(child) &&
(/^h[1-6]$/.test((child.type as string)) || child.type === 'p')
hexart marked this conversation as resolved.
Show resolved Hide resolved
);
},
table({ children, ...props }) {
return (
<Card className="[&:not(:first-child)]:mt-2 [&:not(:last-child)]:mb-2">
<Table {...(props as any)}>{children}</Table>
</Card>

const commonClassNames = "leading-7 [&:not(:first-child)]:mt-4 whitespace-pre-wrap break-words";

return containsBlockElement ? (
<div {...omit(props, ['node'])} className={commonClassNames} />
) : (
<p {...omit(props, ['node'])} className={commonClassNames} />
);
},
thead({ children, ...props }) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const MessageContent = memo(
const isMessage = message.type.includes('message');

const outputMarkdown = (
<p className="flex flex-col gap-2">
<div className="flex flex-col gap-2"> {/* change to div */}
hexart marked this conversation as resolved.
Show resolved Hide resolved
{!isMessage && displayInput ? (
<div className="text-lg font-semibold leading-none tracking-tight">
Output
Expand All @@ -51,7 +51,7 @@ const MessageContent = memo(
>
{output}
</Markdown>
</p>
</div>
);

let inputMarkdown;
Expand All @@ -73,7 +73,7 @@ const MessageContent = memo(
});

inputMarkdown = (
<p className="flex flex-col gap-2">
<div className="flex flex-col gap-2"> {/* change to div */}
<div className="text-lg font-semibold leading-none tracking-tight">
Input
</div>
Expand All @@ -84,7 +84,7 @@ const MessageContent = memo(
>
{input}
</Markdown>
</p>
</div>
);
}

Expand Down
7 changes: 3 additions & 4 deletions frontend/src/components/chat/Messages/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MessageContext } from 'contexts/MessageContext';
import { memo, useContext } from 'react';
import React, { memo, useContext } from 'react';

import {
type IAction,
Expand Down Expand Up @@ -61,10 +61,9 @@ const Messages = memo(
const scorableRun =
!isRunning && m.name !== 'on_chat_start' ? m : undefined;
return (
<>
<React.Fragment key={m.id}>
{m.steps?.length ? (
<Messages
key={m.id}
messages={m.steps}
elements={elements}
actions={actions}
Expand All @@ -76,7 +75,7 @@ const Messages = memo(
{showToolCoTLoader || showHiddenCoTLoader ? (
<BlinkingCursor />
) : null}
</>
</React.Fragment>
);
} else {
// Score the current run
Expand Down
19 changes: 6 additions & 13 deletions frontend/src/components/header/ChatProfiles.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { size } from 'lodash';
import { useContext, useState } from 'react';
import { useContext, useEffect, useState } from 'react';

import {
ChainlitContext,
Expand Down Expand Up @@ -38,9 +37,11 @@ export default function ChatProfiles({ navigate }: Props) {
const [newChatProfile, setNewChatProfile] = useState<string | null>(null);
const [openDialog, setOpenDialog] = useState(false);

if (!chatProfile && size(config?.chatProfiles) > 0) {
setChatProfile(config?.chatProfiles[0].name);
}
useEffect(() => {
if (!chatProfile && config?.chatProfiles && config.chatProfiles.length > 0) {
setChatProfile(config?.chatProfiles?.[0]?.name);
}
}, [chatProfile, config?.chatProfiles, setChatProfile]);

if (typeof config === 'undefined' || config.chatProfiles.length <= 1) {
return null;
Expand All @@ -59,14 +60,6 @@ export default function ChatProfiles({ navigate }: Props) {
handleClose();
};

if (!chatProfile && config?.chatProfiles?.length > 0) {
setChatProfile(config.chatProfiles[0].name);
}

if (!config || config.chatProfiles.length <= 1) {
return null;
}

const allowHtml = config?.features?.unsafe_allow_html;
const latex = config?.features?.latex;

Expand Down
Loading