Skip to content

Commit 298908f

Browse files
fix: ensure VersionIndicator only shows on welcome screen (RooCodeInc#5288)
Co-authored-by: Daniel Riccio <[email protected]>
1 parent 24eb6e4 commit 298908f

File tree

2 files changed

+127
-29
lines changed

2 files changed

+127
-29
lines changed

webview-ui/src/components/chat/ChatView.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,9 +1367,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
13671367

13681368
return (
13691369
<div className={isHidden ? "hidden" : "fixed top-0 left-0 right-0 bottom-0 flex flex-col overflow-hidden"}>
1370-
{/* Version indicator in top-right corner */}
1371-
<VersionIndicator onClick={() => setShowAnnouncementModal(true)} className="absolute top-2 right-3 z-10" />
1372-
13731370
{(showAnnouncement || showAnnouncementModal) && (
13741371
<Announcement
13751372
hideAnnouncement={() => {
@@ -1410,7 +1407,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
14101407
)}
14111408
</>
14121409
) : (
1413-
<div className="flex-1 min-h-0 overflow-y-auto flex flex-col gap-4">
1410+
<div className="flex-1 min-h-0 overflow-y-auto flex flex-col gap-4 relative">
14141411
{/* Moved Task Bar Header Here */}
14151412
{tasks.length !== 0 && (
14161413
<div className="flex text-vscode-descriptionForeground w-full mx-auto px-5 pt-3">
@@ -1426,6 +1423,12 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
14261423
)}
14271424
<div
14281425
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`}>
1426+
{/* Version indicator in top-right corner - only on welcome screen */}
1427+
<VersionIndicator
1428+
onClick={() => setShowAnnouncementModal(true)}
1429+
className="absolute top-2 right-3 z-10"
1430+
/>
1431+
14291432
<RooHero />
14301433
{telemetrySetting === "unset" && <TelemetryBanner />}
14311434
{/* Show the task history preview if expanded and tasks exist */}

webview-ui/src/components/chat/__tests__/ChatView.spec.tsx

Lines changed: 120 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -61,23 +61,17 @@ vi.mock("../AutoApproveMenu", () => ({
6161
default: () => null,
6262
}))
6363

64-
vi.mock("@src/components/common/VersionIndicator", () => ({
65-
default: function MockVersionIndicator({ onClick, className }: { onClick: () => void; className?: string }) {
66-
// eslint-disable-next-line @typescript-eslint/no-require-imports
67-
const React = require("react")
68-
return React.createElement(
69-
"button",
70-
{
71-
onClick,
72-
className,
73-
"aria-label": "chat:versionIndicator.ariaLabel",
74-
"data-testid": "version-indicator",
75-
},
76-
"v3.21.5",
77-
)
78-
},
64+
// Mock VersionIndicator - returns null by default to prevent rendering in tests
65+
vi.mock("../../common/VersionIndicator", () => ({
66+
default: vi.fn(() => null),
7967
}))
8068

69+
// Get the mock function after the module is mocked
70+
const mockVersionIndicator = vi.mocked(
71+
// @ts-expect-error - accessing mocked module
72+
(await import("../../common/VersionIndicator")).default,
73+
)
74+
8175
vi.mock("@src/components/modals/Announcement", () => ({
8276
default: function MockAnnouncement({ hideAnnouncement }: { hideAnnouncement: () => void }) {
8377
// eslint-disable-next-line @typescript-eslint/no-require-imports
@@ -1121,7 +1115,25 @@ describe("ChatView - Focus Grabbing Tests", () => {
11211115
describe("ChatView - Version Indicator Tests", () => {
11221116
beforeEach(() => vi.clearAllMocks())
11231117

1118+
// Helper function to create a mock VersionIndicator implementation
1119+
const createMockVersionIndicator = (
1120+
ariaLabel: string = "chat:versionIndicator.ariaLabel",
1121+
version: string = "v3.21.5",
1122+
) => {
1123+
return (props?: { onClick?: () => void; className?: string }) => {
1124+
const { onClick, className } = props || {}
1125+
return (
1126+
<button data-testid="version-indicator" onClick={onClick} className={className} aria-label={ariaLabel}>
1127+
{version}
1128+
</button>
1129+
)
1130+
}
1131+
}
1132+
11241133
it("displays version indicator button", () => {
1134+
// Temporarily override the mock for this test
1135+
mockVersionIndicator.mockImplementation(createMockVersionIndicator())
1136+
11251137
const { getByLabelText } = renderChatView()
11261138

11271139
// First hydrate state
@@ -1133,26 +1145,38 @@ describe("ChatView - Version Indicator Tests", () => {
11331145
const versionButton = getByLabelText(/version/i)
11341146
expect(versionButton).toBeInTheDocument()
11351147
expect(versionButton).toHaveTextContent(/^v\d+\.\d+\.\d+/)
1148+
1149+
// Reset mock
1150+
mockVersionIndicator.mockReturnValue(null)
11361151
})
11371152

11381153
it("opens announcement modal when version indicator is clicked", () => {
1139-
const { container } = renderChatView()
1154+
// Temporarily override the mock for this test
1155+
mockVersionIndicator.mockImplementation(createMockVersionIndicator("Version 3.22.5", "v3.22.5"))
1156+
1157+
const { getByTestId } = renderChatView()
11401158

11411159
// First hydrate state
11421160
mockPostMessage({
11431161
clineMessages: [],
11441162
})
11451163

11461164
// Find version indicator
1147-
const versionButton = container.querySelector('button[aria-label*="version"]') as HTMLButtonElement
1148-
expect(versionButton).toBeTruthy()
1165+
const versionButton = getByTestId("version-indicator")
1166+
expect(versionButton).toBeInTheDocument()
11491167

11501168
// Click should trigger modal - we'll just verify the button exists and is clickable
11511169
// The actual modal rendering is handled by the component state
11521170
expect(versionButton.onclick).toBeDefined()
1171+
1172+
// Reset mock
1173+
mockVersionIndicator.mockReturnValue(null)
11531174
})
11541175

11551176
it("version indicator has correct styling classes", () => {
1177+
// Temporarily override the mock for this test
1178+
mockVersionIndicator.mockImplementation(createMockVersionIndicator("Version 3.22.5", "v3.22.5"))
1179+
11561180
const { getByTestId } = renderChatView()
11571181

11581182
// First hydrate state
@@ -1165,21 +1189,92 @@ describe("ChatView - Version Indicator Tests", () => {
11651189
expect(versionButton).toBeInTheDocument()
11661190
// The className is passed as a prop to VersionIndicator
11671191
expect(versionButton.className).toContain("absolute top-2 right-3 z-10")
1192+
1193+
// Reset mock
1194+
mockVersionIndicator.mockReturnValue(null)
11681195
})
11691196

11701197
it("version indicator has proper accessibility attributes", () => {
1171-
const { container } = renderChatView()
1198+
// Temporarily override the mock for this test
1199+
mockVersionIndicator.mockImplementation(createMockVersionIndicator("Version 3.22.5", "v3.22.5"))
1200+
1201+
const { getByTestId } = renderChatView()
11721202

11731203
// First hydrate state
11741204
mockPostMessage({
11751205
clineMessages: [],
11761206
})
11771207

1178-
// Check accessibility - find button by its content
1179-
const versionButton = container.querySelector('button[aria-label*="version"]')
1180-
expect(versionButton).toBeTruthy()
1181-
expect(versionButton).toHaveAttribute("aria-label")
1182-
// The mock returns the key, so we check for that
1183-
expect(versionButton?.getAttribute("aria-label")).toBe("chat:versionIndicator.ariaLabel")
1208+
// Check accessibility
1209+
const versionButton = getByTestId("version-indicator")
1210+
expect(versionButton).toBeInTheDocument()
1211+
expect(versionButton).toHaveAttribute("aria-label", "Version 3.22.5")
1212+
1213+
// Reset mock
1214+
mockVersionIndicator.mockReturnValue(null)
1215+
})
1216+
1217+
it("does not display version indicator when there is an active task", () => {
1218+
const { queryByTestId } = renderChatView()
1219+
1220+
// Hydrate state with an active task - any message in the array makes task truthy
1221+
mockPostMessage({
1222+
clineMessages: [
1223+
{
1224+
type: "say",
1225+
say: "task",
1226+
ts: Date.now(),
1227+
text: "Active task in progress",
1228+
},
1229+
],
1230+
})
1231+
1232+
// Version indicator should not be present during task execution
1233+
const versionButton = queryByTestId("version-indicator")
1234+
expect(versionButton).not.toBeInTheDocument()
1235+
})
1236+
1237+
it("displays version indicator only on welcome screen (no task)", () => {
1238+
// Temporarily override the mock for this test
1239+
mockVersionIndicator.mockImplementation(createMockVersionIndicator("Version 3.22.5", "v3.22.5"))
1240+
1241+
const { queryByTestId, rerender } = renderChatView()
1242+
1243+
// First, hydrate with no messages (welcome screen)
1244+
mockPostMessage({
1245+
clineMessages: [],
1246+
})
1247+
1248+
// Version indicator should be present
1249+
let versionButton = queryByTestId("version-indicator")
1250+
expect(versionButton).toBeInTheDocument()
1251+
1252+
// Reset mock to return null for the second part of the test
1253+
mockVersionIndicator.mockReturnValue(null)
1254+
1255+
// Now add a task - any message makes task truthy
1256+
mockPostMessage({
1257+
clineMessages: [
1258+
{
1259+
type: "say",
1260+
say: "task",
1261+
ts: Date.now(),
1262+
text: "Starting a new task",
1263+
},
1264+
],
1265+
})
1266+
1267+
// Force a re-render to ensure the component updates
1268+
rerender(
1269+
<ExtensionStateContextProvider>
1270+
<QueryClientProvider client={queryClient}>
1271+
<ChatView {...defaultProps} />
1272+
</QueryClientProvider>
1273+
</ExtensionStateContextProvider>,
1274+
)
1275+
1276+
// Version indicator should disappear
1277+
versionButton = queryByTestId("version-indicator")
1278+
expect(versionButton).not.toBeInTheDocument()
11841279
})
11851280
})

0 commit comments

Comments
 (0)