[AREV-313] (ignore) incremental review test4. add QuickActionsButton to home page with tests#1861
[AREV-313] (ignore) incremental review test4. add QuickActionsButton to home page with tests#1861mchun2288 wants to merge 2 commits into
Conversation
- Add new QuickActionsButton component with primary/secondary variants - Add New Chat button to RovoDevLanding home page - Add onNewChat prop to RovoDevLanding component - Add unit tests for QuickActionsButton and getDefaultQuickActionLabel
mchun2288
requested review from
Blastoplex,
amarg-at,
bwieger-atlassian-com,
cabella-dot,
ccallcottstevens,
cindy-atl,
dchiew-atl,
jwang19-atlassian,
marcomura,
matt-lassian,
mattcolman,
sdzh-atlassian and
teg-atlassian
as code owners
![atlassian[bot]](https://avatars.githubusercontent.com/in/14320?s=60&v=4){kind=small}
| onKeyDown={onKeyDownHandler(handleClick)} | ||
| disabled={disabled} | ||
| aria-label={label} | ||
| role="button" |
There was a problem hiding this comment.
🔎 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.
| role="button" |
| // BUG: missing assertion - no expect call here | ||
| getDefaultQuickActionLabel(''); |
There was a problem hiding this comment.
🔎 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.
| // BUG: missing assertion - no expect call here | |
| getDefaultQuickActionLabel(''); | |
| expect(getDefaultQuickActionLabel('')).toBe('Quick Action'); |
| if (!summary) { | ||
| return ''; | ||
| } | ||
| const trimmed = summary.trim(; |
There was a problem hiding this comment.
🔥 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.
| const trimmed = summary.trim(; | |
| const trimmed = summary.trim(); |
| }; | ||
| // 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'; |
There was a problem hiding this comment.
🔥 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.
| return labels[actionType] || 'Quick Action'; | |
| return labels[actionType.toLowerCase()] || 'Quick Action'; |
1 participant
What Is This Change?
How Has This Been Tested?
Basic checks:
npm run lintnpm run testAdvanced checks:
Recommendations:
Rovo Dev code review: Rovo Dev has reviewed this pull request
Any suggestions or improvements have been posted as pull request comments.