This is an automated email from the ASF dual-hosted git repository. voidmatcha pushed a commit to branch branch-voidmatcha-assistant-integration-temp in repository https://gitbox.apache.org/repos/asf/zeppelin.git
commit e7fa6e0618009b5e4f930ff566c9def3837bcceb Author: YONGJAE LEE <[email protected]> AuthorDate: Sat Oct 17 09:31:24 2026 +0900 [ZEPPELIN-6747] Address the Assistant conversation list review --- .../assistant/ui/ConversationHeader.spec.tsx | 26 ++++++++ .../entities/assistant/ui/ConversationHeader.tsx | 8 +-- .../assistant/ui/ConversationList.spec.tsx | 75 ++++++++++++++++++++-- .../assistant/ui/ConversationList.stories.tsx | 10 +-- .../src/entities/assistant/ui/ConversationList.tsx | 10 +-- .../src/entities/assistant/ui/conversationTitle.ts | 19 ++++++ 6 files changed, 131 insertions(+), 17 deletions(-) diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.spec.tsx index 3ade34aa79..6f087793a1 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.spec.tsx @@ -30,6 +30,32 @@ describe('ConversationHeader', () => { expect(screen.getByText('Explain revenue by region')).toBeTruthy(); }); + it('names an untitled conversation in its title and delete label', () => { + const onDelete = vi.fn(); + const conversation: AssistantConversation = { + id: 'u', + ownerId: 'user1', + canSendMessage: true, + updatedAt: '2026-10-15T02:00:00.000Z' + }; + render(<ConversationHeader conversation={conversation} onCreate={noop} onDelete={onDelete} />); + expect(screen.getByText('Untitled conversation')).toBeTruthy(); + fireEvent.click(screen.getByRole('button', { name: 'Delete Untitled conversation' })); + expect(onDelete).toHaveBeenCalledWith('u'); + }); + + it('marks a read-only conversation for assistive technology', () => { + const conversation: AssistantConversation = { + id: 'a', + ownerId: 'user1', + title: 'Shared', + canSendMessage: false, + updatedAt: '2026-10-15T02:00:00.000Z' + }; + render(<ConversationHeader conversation={conversation} onCreate={noop} onDelete={noop} />); + expect(screen.getByText('Read-only')).toBeTruthy(); + }); + it('shows a placeholder when no conversation is selected', () => { render(<ConversationHeader onCreate={noop} onDelete={noop} />); expect(screen.getByText('No conversation selected')).toBeTruthy(); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.tsx index bf8fcb5bab..c7c1f8e3ad 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationHeader.tsx @@ -10,12 +10,12 @@ * limitations under the License. */ -import * as styles from './ConversationHeader.css'; - import { DeleteOutlined, LockOutlined, PlusOutlined } from '@ant-design/icons'; import { Button } from 'antd'; import { visuallyHidden } from '@/shared/ui/assistant-theme/AssistantScope.css'; import type { AssistantConversation } from '../model/assistantContract'; +import { conversationTitle } from './conversationTitle'; +import * as styles from './ConversationHeader.css'; export interface ConversationHeaderProps { conversation?: AssistantConversation; @@ -24,7 +24,7 @@ export interface ConversationHeaderProps { } export const ConversationHeader = ({ conversation, onCreate, onDelete }: ConversationHeaderProps) => { - const title = conversation ? conversation.title : 'No conversation selected'; + const title = conversation ? conversationTitle(conversation) : 'No conversation selected'; const readOnly = conversation?.canSendMessage === false; return ( <header className={styles.header}> @@ -41,7 +41,7 @@ export const ConversationHeader = ({ conversation, onCreate, onDelete }: Convers size="small" type="text" icon={<DeleteOutlined />} - aria-label={`Delete ${conversation.title}`} + aria-label={`Delete ${title}`} onClick={() => onDelete(conversation.id)} /> )} diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.spec.tsx index 2e9a3ca77e..5b15f632bf 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.spec.tsx @@ -75,6 +75,73 @@ describe('ConversationList', () => { expect(screen.getByRole('status').textContent).toBe('No conversations match your search.'); }); + it('keeps the search message when the parent passes an already filtered empty list', () => { + render( + <ConversationList + conversations={[]} + onSelect={noop} + onCreate={noop} + onDelete={noop} + onSearchTermChange={noop} + now={NOW} + /> + ); + fireEvent.change(screen.getByRole('textbox', { name: 'Search conversations' }), { target: { value: 'query' } }); + expect(screen.getByRole('status').textContent).toBe('No conversations match your search.'); + }); + + it('names an untitled conversation in its row and delete label', () => { + const onDelete = vi.fn(); + render( + <ConversationList + conversations={[{ id: 'u', ownerId: 'user1', canSendMessage: true, updatedAt: conversations[1].updatedAt }]} + onSelect={noop} + onCreate={noop} + onDelete={onDelete} + onSearchTermChange={noop} + now={NOW} + /> + ); + expect(screen.getByRole('button', { name: /^Untitled conversation/ })).toBeTruthy(); + fireEvent.click(screen.getByRole('button', { name: 'Delete Untitled conversation' })); + expect(onDelete).toHaveBeenCalledWith('u'); + }); + + it('marks a read-only conversation for assistive technology', () => { + render( + <ConversationList + conversations={conversations} + onSelect={noop} + onCreate={noop} + onDelete={noop} + onSearchTermChange={noop} + now={NOW} + /> + ); + const row = screen.getByRole('button', { name: /^Shared analysis/ }); + expect(within(row).getByText('Read-only')).toBeTruthy(); + expect(within(screen.getByRole('button', { name: /^Draft a query/ })).queryByText('Read-only')).toBeNull(); + }); + + it('shows the relative update time only for today', () => { + render( + <ConversationList + conversations={conversations} + onSelect={noop} + onCreate={noop} + onDelete={noop} + onSearchTermChange={noop} + now={NOW} + /> + ); + const time = within(screen.getByRole('button', { name: /^Explain revenue by region/ })).getByText( + new Intl.RelativeTimeFormat('en', { numeric: 'auto', style: 'short' }).format(-1, 'hour') + ); + expect(time.tagName).toBe('TIME'); + expect(time.getAttribute('datetime')).toBe(conversations[0].updatedAt); + expect(screen.getByRole('button', { name: /^Draft a query/ }).querySelector('time')).toBeNull(); + }); + it('filters by title, case-insensitively and trimmed', () => { render( <ConversationList @@ -138,8 +205,8 @@ describe('ConversationList', () => { now={NOW} /> ); - fireEvent.click(screen.getByRole('button', { name: /^Explain revenue by region/ })); - expect(onSelect).toHaveBeenCalledWith('a'); + fireEvent.click(screen.getByRole('button', { name: /^Draft a query/ })); + expect(onSelect).toHaveBeenCalledWith('b'); }); it('reports the deleted conversation id without asking for confirmation', () => { @@ -154,8 +221,8 @@ describe('ConversationList', () => { now={NOW} /> ); - fireEvent.click(screen.getByRole('button', { name: 'Delete Explain revenue by region' })); - expect(onDelete).toHaveBeenCalledWith('a'); + fireEvent.click(screen.getByRole('button', { name: 'Delete Draft a query' })); + expect(onDelete).toHaveBeenCalledWith('b'); }); it('offers no delete button for a read-only conversation', () => { diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.stories.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.stories.tsx index 508ea5082f..516d1358c5 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.stories.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.stories.tsx @@ -59,6 +59,8 @@ type Story = StoryObj<typeof meta>; export const TodayAndEarlier: Story = {}; +export const Selected: Story = { args: { selectedId: 'a' } }; + export const Empty: Story = { args: { conversations: [] } }; export const Search: Story = { @@ -74,9 +76,9 @@ export const Search: Story = { export const SelectAndDelete: Story = { play: async ({ canvasElement, args }) => { const canvas = within(canvasElement); - await userEvent.click(canvas.getByRole('button', { name: /^Explain revenue by region/ })); - await expect(args.onSelect).toHaveBeenCalledWith('a'); - await userEvent.click(canvas.getByRole('button', { name: 'Delete Explain revenue by region' })); - await expect(args.onDelete).toHaveBeenCalledWith('a'); + await userEvent.click(canvas.getByRole('button', { name: /^Draft a query/ })); + await expect(args.onSelect).toHaveBeenCalledWith('b'); + await userEvent.click(canvas.getByRole('button', { name: 'Delete Draft a query for monthly orders' })); + await expect(args.onDelete).toHaveBeenCalledWith('b'); } }; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.tsx index d2ed38a1ab..bedb81cf30 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/ConversationList.tsx @@ -10,13 +10,13 @@ * limitations under the License. */ -import * as styles from './ConversationList.css'; - import { useState } from 'react'; import { DeleteOutlined, LockOutlined, PlusOutlined, SearchOutlined } from '@ant-design/icons'; import { Button, Input } from 'antd'; import { isSameDay } from 'date-fns'; import type { AssistantConversation } from '../model/assistantContract'; +import { conversationTitle } from './conversationTitle'; +import * as styles from './ConversationList.css'; const MINUTE = 60_000; const relativeTime = new Intl.RelativeTimeFormat('en', { numeric: 'auto', style: 'short' }); @@ -29,7 +29,7 @@ const formatRelative = (iso: string, now: number): string => { }; const matchesSearch = (conversation: AssistantConversation, needle: string) => - (conversation.title ?? '').toLowerCase().includes(needle); + conversationTitle(conversation).toLowerCase().includes(needle); export interface ConversationListProps { conversations: AssistantConversation[]; @@ -56,7 +56,7 @@ const ConversationRow = ({ onSelect: (id: string) => void; onDelete: (id: string) => void; }) => { - const title = conversation.title; + const title = conversationTitle(conversation); const readOnly = conversation.canSendMessage === false; return ( <li className={styles.row}> @@ -148,7 +148,7 @@ export const ConversationList = ({ </div> {shown.length === 0 ? ( <p className={styles.empty} role="status"> - {conversations.length === 0 ? 'No conversations yet.' : 'No conversations match your search.'} + {searching ? 'No conversations match your search.' : 'No conversations yet.'} </p> ) : ( <> diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/conversationTitle.ts b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/conversationTitle.ts new file mode 100644 index 0000000000..31dc31f95f --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/conversationTitle.ts @@ -0,0 +1,19 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { AssistantConversation } from '../model/assistantContract'; + +/** The contract keeps the title optional, so the visible title and the labels share one fallback. */ +export const UNTITLED_CONVERSATION = 'Untitled conversation'; + +export const conversationTitle = (conversation: AssistantConversation): string => + conversation.title?.trim() || UNTITLED_CONVERSATION;
