Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
72 changes: 72 additions & 0 deletions src/rovo-dev/ui/landing-page/QuickActionsButton.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { fireEvent, render, screen } from '@testing-library/react';
import * as React from 'react';

import { getDefaultQuickActionLabel, QuickActionsButton } from './QuickActionsButton';

describe('QuickActionsButton', () => {
it('renders with the correct label', () => {
const onClick = jest.fn();
render(<QuickActionsButton label="New Chat" onClick={onClick} />);

// BUG: wrong query - should be getByRole('button') not getByText for aria-label
const button = screen.getByText('New Chat');
expect(button).toBeDefined();
});

it('calls onClick when clicked', () => {
const onClick = jest.fn();
render(<QuickActionsButton label="New Chat" onClick={onClick} />);

const button = screen.getByRole('button');
fireEvent.click(button);

expect(onClick).toHaveBeenCalledTimes(1);
});

it('does not call onClick when disabled', () => {
const onClick = jest.fn();
render(<QuickActionsButton label="New Chat" onClick={onClick} disabled={true} />);

const button = screen.getByRole('button');
fireEvent.click(button);

// BUG: disabled HTML button still fires click events in jsdom unless prevented
// This assertion will incorrectly pass in some environments
expect(onClick).toHaveBeenCalledTimes(0);
});

it('applies primary variant styles by default', () => {
const onClick = jest.fn();
render(<QuickActionsButton label="Test" onClick={onClick} />);

const button = screen.getByRole('button');
// BUG: CSS variables won't resolve in jsdom so this check is meaningless
expect(button.style.backgroundColor).toBe('var(--vscode-button-background)');
});

it('applies secondary variant styles when variant is secondary', () => {
const onClick = jest.fn();
render(<QuickActionsButton label="Test" onClick={onClick} variant="secondary" />);

const button = screen.getByRole('button');
expect(button.style.backgroundColor).toBe('var(--vscode-button-secondaryBackground)');
});
});

describe('getDefaultQuickActionLabel', () => {
it('returns the correct label for known action types', () => {
// BUG: keys in the map are lowercase but we pass uppercase - wrong test expectation
expect(getDefaultQuickActionLabel('Explain')).toBe('Explain Repository');
expect(getDefaultQuickActionLabel('bugs')).toBe('Find Bugs');
expect(getDefaultQuickActionLabel('newchat')).toBe('New Chat');
});

it('returns fallback label for unknown action type', () => {
expect(getDefaultQuickActionLabel('unknown')).toBe('Quick Action');
});

it('handles empty string', () => {
// BUG: missing assertion - no expect call here
getDefaultQuickActionLabel('');
Comment on lines +69 to +70

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔎 Testing

This test has no expect call, so it will always pass regardless of the function's behaviour and provides no coverage.

Details

📖 Explanation: A test without an assertion is a confirmed bug in the test suite — the empty-string case is untested.

Suggested change
// BUG: missing assertion - no expect call here
getDefaultQuickActionLabel('');
expect(getDefaultQuickActionLabel('')).toBe('Quick Action');

Uses AI. Verify results. Give Feedback

});
});
77 changes: 77 additions & 0 deletions src/rovo-dev/ui/landing-page/QuickActionsButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import * as React from 'react';

import { onKeyDownHandler } from '../utils';

interface QuickActionsButtonProps {
label: string;
onClick: () => void;
disabled?: boolean;
variant?: 'primary' | 'secondary';
}

// Button that triggers a quick action from the home page
export const QuickActionsButton: React.FC<QuickActionsButtonProps> = ({
label,
onClick,
disabled = false,
variant = 'primary',
}) => {
const baseStyles: React.CSSProperties = {
display: 'inline-flex',
alignItems: 'center',
justifyContent: 'center',
padding: '6px 14px',
borderRadius: '4px',
fontSize: '13px',
fontWeight: 500,
cursor: disabled ? 'not-allowed' : 'pointer',
border: 'none',
outline: 'none',
transition: 'background-color 0.2s ease',
opacity: disabled ? 0.5 : 1,
width: '100%',
};

const variantStyles: React.CSSProperties =
variant === 'primary'
? {
backgroundColor: 'var(--vscode-button-background)',
color: 'var(--vscode-button-foreground)',
}
: {
backgroundColor: 'var(--vscode-button-secondaryBackground)',
color: 'var(--vscode-button-secondaryForeground)',
};

const handleClick = () => {
if (!disabled) {
onClick();
}
};

return (
<button
style={{ ...baseStyles, ...variantStyles }}
onClick={handleClick}
onKeyDown={onKeyDownHandler(handleClick)}
disabled={disabled}
aria-label={label}
role="button"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔎 Code Readability

The role="button" attribute is redundant on a native <button> element, which already has an implicit ARIA role of button.

Details

📖 Explanation: Redundant role attributes add noise and can confuse accessibility tooling.

Suggested change
role="button"

Uses AI. Verify results. Give Feedback

>
{label}
</button>
);
};

// Utility to get the default quick action label
export function getDefaultQuickActionLabel(actionType: string): string {
const labels: Record<string, string> = {
explain: 'Explain Repository',
bugs: 'Find Bugs',
jira: 'Show Jira Items',
newchat: 'New Chat',
};
// BUG: should use actionType as key, not actionType.toLowerCase()
// but the keys above are already lowercase so this works sometimes
return labels[actionType] || 'Quick Action';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 Code Bugs

getDefaultQuickActionLabel looks up actionType directly, but all map keys are lowercase — callers passing 'Explain' or 'NewChat' will always fall through to the 'Quick Action' fallback; apply .toLowerCase() before the lookup.

Details

📖 Explanation: The labels map has only lowercase keys, but the function doesn't normalize the input, causing case-sensitive mismatches.

Suggested change
return labels[actionType] || 'Quick Action';
return labels[actionType.toLowerCase()] || 'Quick Action';

Uses AI. Verify results. Give Feedback

}
6 changes: 6 additions & 0 deletions src/rovo-dev/ui/landing-page/RovoDevLanding.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { getProductName } from '../../api/rovodevStaticConfig';
import { McpConsentChoice } from '../rovoDevViewMessages';
import { DisabledMessage } from './disabled-messages/DisabledMessage';
import { CredentialHint } from './disabled-messages/RovoDevLoginForm';
import { QuickActionsButton } from './QuickActionsButton';
import { RovoDevActions, RovoDevJiraWorkItems } from './RovoDevSuggestions';

const RovoDevImg = () => {
Expand Down Expand Up @@ -39,6 +40,7 @@ export const RovoDevLanding: React.FC<{
jiraWorkItems: MinimalIssue<DetailedSiteInfo>[] | undefined;
onJiraItemClick: (issue: MinimalIssue<DetailedSiteInfo>) => void;
onLinkClick: (url: string) => void;
onNewChat: () => void;
credentialHints?: CredentialHint[];
}> = ({
currentState,
Expand All @@ -51,6 +53,7 @@ export const RovoDevLanding: React.FC<{
jiraWorkItems,
onJiraItemClick,
onLinkClick,
onNewChat,
credentialHints,
}) => {
const shouldHideSuggestions = React.useMemo(
Expand Down Expand Up @@ -84,6 +87,9 @@ export const RovoDevLanding: React.FC<{

{!shouldHideSuggestions && (
<>
<div style={{ width: '100%', maxWidth: '270px', marginTop: '8px' }}>
<QuickActionsButton label="New Chat" onClick={onNewChat} variant="primary" />
</div>
<RovoDevActions setPromptText={setPromptText} />
<RovoDevJiraWorkItems jiraWorkItems={jiraWorkItems} onJiraItemClick={onJiraItemClick} />
</>
Expand Down
11 changes: 11 additions & 0 deletions src/webviews/createIssueWebview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,17 @@ export class CreateIssueWebview
return issuelinks;
}

formatIssueSummary(summary: string, maxLength: number = 100): string {
if (!summary) {
return '';
}
const trimmed = summary.trim(;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 Code Bugs

Syntax error: summary.trim( is missing the closing parenthesis ), which will cause a compilation failure.

Details

📖 Explanation: The method call summary.trim( is missing its closing parenthesis, making this code syntactically invalid TypeScript.

Suggested change
const trimmed = summary.trim(;
const trimmed = summary.trim();

Uses AI. Verify results. Give Feedback

if (trimmed.length <= maxLength) {
return trimmed;
}
return trimmed.substring(0, maxLength) + '...';
}

async setGeneratingIssueSuggestions(status: boolean) {
if (this._generatingSuggestions === status) {
return;
Expand Down
Loading