fix: ensure VersionIndicator only shows on welcome screen (#5288)

Co-authored-by: Daniel Riccio <ricciodaniel98@gmail.com>
This commit is contained in:
Hannes Rudolph 2025-06-30 12:07:00 -06:00 committed by GitHub
parent 24eb6e4c29
commit 298908fe1d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 127 additions and 29 deletions

View file

@ -1367,9 +1367,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
return (
<div className={isHidden ? "hidden" : "fixed top-0 left-0 right-0 bottom-0 flex flex-col overflow-hidden"}>
{/* Version indicator in top-right corner */}
<VersionIndicator onClick={() => setShowAnnouncementModal(true)} className="absolute top-2 right-3 z-10" />
{(showAnnouncement || showAnnouncementModal) && (
<Announcement
hideAnnouncement={() => {
@ -1410,7 +1407,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
)}
</>
) : (
<div className="flex-1 min-h-0 overflow-y-auto flex flex-col gap-4">
<div className="flex-1 min-h-0 overflow-y-auto flex flex-col gap-4 relative">
{/* Moved Task Bar Header Here */}
{tasks.length !== 0 && (
<div className="flex text-vscode-descriptionForeground w-full mx-auto px-5 pt-3">
@ -1426,6 +1423,12 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
)}
<div
className={` w-full flex flex-col gap-4 m-auto ${isExpanded && tasks.length > 0 ? "mt-0" : ""} px-3.5 min-[370px]:px-10 pt-5 transition-all duration-300`}>
{/* Version indicator in top-right corner - only on welcome screen */}
<VersionIndicator
onClick={() => setShowAnnouncementModal(true)}
className="absolute top-2 right-3 z-10"
/>
<RooHero />
{telemetrySetting === "unset" && <TelemetryBanner />}
{/* Show the task history preview if expanded and tasks exist */}

View file

@ -61,23 +61,17 @@ vi.mock("../AutoApproveMenu", () => ({
default: () => null,
}))
vi.mock("@src/components/common/VersionIndicator", () => ({
default: function MockVersionIndicator({ onClick, className }: { onClick: () => void; className?: string }) {
// eslint-disable-next-line @typescript-eslint/no-require-imports
const React = require("react")
return React.createElement(
"button",
{
onClick,
className,
"aria-label": "chat:versionIndicator.ariaLabel",
"data-testid": "version-indicator",
},
"v3.21.5",
)
},
// Mock VersionIndicator - returns null by default to prevent rendering in tests
vi.mock("../../common/VersionIndicator", () => ({
default: vi.fn(() => null),
}))
// Get the mock function after the module is mocked
const mockVersionIndicator = vi.mocked(
// @ts-expect-error - accessing mocked module
(await import("../../common/VersionIndicator")).default,
)
vi.mock("@src/components/modals/Announcement", () => ({
default: function MockAnnouncement({ hideAnnouncement }: { hideAnnouncement: () => void }) {
// eslint-disable-next-line @typescript-eslint/no-require-imports
@ -1121,7 +1115,25 @@ describe("ChatView - Focus Grabbing Tests", () => {
describe("ChatView - Version Indicator Tests", () => {
beforeEach(() => vi.clearAllMocks())
// Helper function to create a mock VersionIndicator implementation
const createMockVersionIndicator = (
ariaLabel: string = "chat:versionIndicator.ariaLabel",
version: string = "v3.21.5",
) => {
return (props?: { onClick?: () => void; className?: string }) => {
const { onClick, className } = props || {}
return (
<button data-testid="version-indicator" onClick={onClick} className={className} aria-label={ariaLabel}>
{version}
</button>
)
}
}
it("displays version indicator button", () => {
// Temporarily override the mock for this test
mockVersionIndicator.mockImplementation(createMockVersionIndicator())
const { getByLabelText } = renderChatView()
// First hydrate state
@ -1133,10 +1145,16 @@ describe("ChatView - Version Indicator Tests", () => {
const versionButton = getByLabelText(/version/i)
expect(versionButton).toBeInTheDocument()
expect(versionButton).toHaveTextContent(/^v\d+\.\d+\.\d+/)
// Reset mock
mockVersionIndicator.mockReturnValue(null)
})
it("opens announcement modal when version indicator is clicked", () => {
const { container } = renderChatView()
// Temporarily override the mock for this test
mockVersionIndicator.mockImplementation(createMockVersionIndicator("Version 3.22.5", "v3.22.5"))
const { getByTestId } = renderChatView()
// First hydrate state
mockPostMessage({
@ -1144,15 +1162,21 @@ describe("ChatView - Version Indicator Tests", () => {
})
// Find version indicator
const versionButton = container.querySelector('button[aria-label*="version"]') as HTMLButtonElement
expect(versionButton).toBeTruthy()
const versionButton = getByTestId("version-indicator")
expect(versionButton).toBeInTheDocument()
// Click should trigger modal - we'll just verify the button exists and is clickable
// The actual modal rendering is handled by the component state
expect(versionButton.onclick).toBeDefined()
// Reset mock
mockVersionIndicator.mockReturnValue(null)
})
it("version indicator has correct styling classes", () => {
// Temporarily override the mock for this test
mockVersionIndicator.mockImplementation(createMockVersionIndicator("Version 3.22.5", "v3.22.5"))
const { getByTestId } = renderChatView()
// First hydrate state
@ -1165,21 +1189,92 @@ describe("ChatView - Version Indicator Tests", () => {
expect(versionButton).toBeInTheDocument()
// The className is passed as a prop to VersionIndicator
expect(versionButton.className).toContain("absolute top-2 right-3 z-10")
// Reset mock
mockVersionIndicator.mockReturnValue(null)
})
it("version indicator has proper accessibility attributes", () => {
const { container } = renderChatView()
// Temporarily override the mock for this test
mockVersionIndicator.mockImplementation(createMockVersionIndicator("Version 3.22.5", "v3.22.5"))
const { getByTestId } = renderChatView()
// First hydrate state
mockPostMessage({
clineMessages: [],
})
// Check accessibility - find button by its content
const versionButton = container.querySelector('button[aria-label*="version"]')
expect(versionButton).toBeTruthy()
expect(versionButton).toHaveAttribute("aria-label")
// The mock returns the key, so we check for that
expect(versionButton?.getAttribute("aria-label")).toBe("chat:versionIndicator.ariaLabel")
// Check accessibility
const versionButton = getByTestId("version-indicator")
expect(versionButton).toBeInTheDocument()
expect(versionButton).toHaveAttribute("aria-label", "Version 3.22.5")
// Reset mock
mockVersionIndicator.mockReturnValue(null)
})
it("does not display version indicator when there is an active task", () => {
const { queryByTestId } = renderChatView()
// Hydrate state with an active task - any message in the array makes task truthy
mockPostMessage({
clineMessages: [
{
type: "say",
say: "task",
ts: Date.now(),
text: "Active task in progress",
},
],
})
// Version indicator should not be present during task execution
const versionButton = queryByTestId("version-indicator")
expect(versionButton).not.toBeInTheDocument()
})
it("displays version indicator only on welcome screen (no task)", () => {
// Temporarily override the mock for this test
mockVersionIndicator.mockImplementation(createMockVersionIndicator("Version 3.22.5", "v3.22.5"))
const { queryByTestId, rerender } = renderChatView()
// First, hydrate with no messages (welcome screen)
mockPostMessage({
clineMessages: [],
})
// Version indicator should be present
let versionButton = queryByTestId("version-indicator")
expect(versionButton).toBeInTheDocument()
// Reset mock to return null for the second part of the test
mockVersionIndicator.mockReturnValue(null)
// Now add a task - any message makes task truthy
mockPostMessage({
clineMessages: [
{
type: "say",
say: "task",
ts: Date.now(),
text: "Starting a new task",
},
],
})
// Force a re-render to ensure the component updates
rerender(
<ExtensionStateContextProvider>
<QueryClientProvider client={queryClient}>
<ChatView {...defaultProps} />
</QueryClientProvider>
</ExtensionStateContextProvider>,
)
// Version indicator should disappear
versionButton = queryByTestId("version-indicator")
expect(versionButton).not.toBeInTheDocument()
})
})