Skip to content

Commit d73d42d

Browse files
Copilotalexr00
andcommitted
Make Comment the primary action in review dropdown with proper ordering and styling
Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent 5ccdcd3 commit d73d42d

File tree

2 files changed

+21
-12
lines changed

2 files changed

+21
-12
lines changed

webviews/components/comment.tsx

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,8 @@ export function AddComment({
349349
isIssue,
350350
isAuthor,
351351
continueOnGitHub,
352-
currentUserReviewState,
353-
lastReviewType,
352+
currentUserReviewState: _currentUserReviewState,
353+
lastReviewType: _lastReviewType,
354354
busy,
355355
hasReviewDraft,
356356
}: PullRequest) {
@@ -372,7 +372,8 @@ export function AddComment({
372372
close(value);
373373
};
374374

375-
let currentSelection: ReviewType = lastReviewType ?? (currentUserReviewState === 'APPROVED' ? ReviewType.Approve : (currentUserReviewState === 'CHANGES_REQUESTED' ? ReviewType.RequestChanges : ReviewType.Comment));
375+
// Always use Comment as the primary action
376+
let currentSelection: ReviewType = ReviewType.Comment;
376377

377378
async function submitAction(action: ReviewType): Promise<void> {
378379
const { value } = textareaRef.current!;
@@ -460,12 +461,13 @@ export function AddComment({
460461
defaultOptionValue={() => currentSelection}
461462
allOptions={() => {
462463
const actions: { label: string; value: string; optionDisabled: boolean; action: (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) => void }[] = [];
463-
if (availableActions.approve) {
464-
actions.push({ label: availableActions[ReviewType.Approve]!, value: ReviewType.Approve, action: () => submitAction(ReviewType.Approve), optionDisabled: shouldDisableApproveButton });
465-
}
464+
// Comment is always the primary action and should appear first
466465
if (availableActions.comment) {
467466
actions.push({ label: availableActions[ReviewType.Comment]!, value: ReviewType.Comment, action: () => submitAction(ReviewType.Comment), optionDisabled: shouldDisableNonApproveButtons });
468467
}
468+
if (availableActions.approve) {
469+
actions.push({ label: availableActions[ReviewType.Approve]!, value: ReviewType.Approve, action: () => submitAction(ReviewType.Approve), optionDisabled: shouldDisableApproveButton });
470+
}
469471
if (availableActions.requestChanges) {
470472
actions.push({ label: availableActions[ReviewType.RequestChanges]!, value: ReviewType.RequestChanges, action: () => submitAction(ReviewType.RequestChanges), optionDisabled: shouldDisableNonApproveButtons });
471473
}
@@ -475,6 +477,7 @@ export function AddComment({
475477
disabled={isBusy || busy}
476478
hasSingleAction={Object.keys(availableActions).length === 1}
477479
spreadable={true}
480+
primaryOptionValue={ReviewType.Comment}
478481
/>
479482
</div>
480483
</form>
@@ -532,7 +535,8 @@ export const AddCommentSimple = (pr: PullRequest) => {
532535
const { updatePR, requestChanges, approve, submit, openOnGitHub } = useContext(PullRequestContext);
533536
const [isBusy, setBusy] = useState(false);
534537
const textareaRef = useRef<HTMLTextAreaElement>();
535-
let currentSelection: ReviewType = pr.lastReviewType ?? (pr.currentUserReviewState === 'APPROVED' ? ReviewType.Approve : (pr.currentUserReviewState === 'CHANGES_REQUESTED' ? ReviewType.RequestChanges : ReviewType.Comment));
538+
// Always use Comment as the primary action
539+
let currentSelection: ReviewType = ReviewType.Comment;
536540

537541
async function submitAction(action: ReviewType): Promise<void> {
538542
const { value } = textareaRef.current!;
@@ -608,12 +612,13 @@ export const AddCommentSimple = (pr: PullRequest) => {
608612
defaultOptionValue={() => currentSelection}
609613
allOptions={() => {
610614
const actions: { label: string; value: string; optionDisabled: boolean; action: (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) => void }[] = [];
611-
if (availableActions.approve) {
612-
actions.push({ label: availableActions[ReviewType.Approve]!, value: ReviewType.Approve, action: () => submitAction(ReviewType.Approve), optionDisabled: shouldDisableApproveButton });
613-
}
615+
// Comment is always the primary action and should appear first
614616
if (availableActions.comment) {
615617
actions.push({ label: availableActions[ReviewType.Comment]!, value: ReviewType.Comment, action: () => submitAction(ReviewType.Comment), optionDisabled: shouldDisableNonApproveButtons });
616618
}
619+
if (availableActions.approve) {
620+
actions.push({ label: availableActions[ReviewType.Approve]!, value: ReviewType.Approve, action: () => submitAction(ReviewType.Approve), optionDisabled: shouldDisableApproveButton });
621+
}
617622
if (availableActions.requestChanges) {
618623
actions.push({ label: availableActions[ReviewType.RequestChanges]!, value: ReviewType.RequestChanges, action: () => submitAction(ReviewType.RequestChanges), optionDisabled: shouldDisableNonApproveButtons });
619624
}
@@ -623,6 +628,7 @@ export const AddCommentSimple = (pr: PullRequest) => {
623628
disabled={isBusy || pr.busy}
624629
hasSingleAction={Object.keys(availableActions).length === 1}
625630
spreadable={true}
631+
primaryOptionValue={ReviewType.Comment}
626632
/>
627633
</div>
628634
</span>

webviews/components/contextDropdown.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ interface ContextDropdownProps {
1717
hasSingleAction?: boolean;
1818
spreadable: boolean;
1919
isSecondary?: boolean;
20+
primaryOptionValue?: string;
2021
}
2122

2223
function useWindowSize() {
@@ -32,7 +33,7 @@ function useWindowSize() {
3233
return size;
3334
}
3435

35-
export const ContextDropdown = ({ optionsContext, defaultOptionLabel, defaultOptionValue, defaultAction, allOptions: options, optionsTitle, disabled, hasSingleAction, spreadable, isSecondary }: ContextDropdownProps) => {
36+
export const ContextDropdown = ({ optionsContext, defaultOptionLabel, defaultOptionValue, defaultAction, allOptions: options, optionsTitle, disabled, hasSingleAction, spreadable, isSecondary, primaryOptionValue }: ContextDropdownProps) => {
3637
const [expanded, setExpanded] = useState(false);
3738
const onHideAction = (e: MouseEvent | KeyboardEvent) => {
3839
if (e.target instanceof HTMLElement && e.target.classList.contains('split-right')) {
@@ -56,7 +57,9 @@ export const ContextDropdown = ({ optionsContext, defaultOptionLabel, defaultOpt
5657

5758
return <div className={`dropdown-container${spreadable ? ' spreadable' : ''}`} ref={divRef}>
5859
{divRef.current && spreadable && (divRef.current.clientWidth > 375) && options && !hasSingleAction ? options().map(({ label, value, action, optionDisabled }) => {
59-
return <button className='inlined-dropdown' key={value} title={label} disabled={optionDisabled || disabled} onClick={action} value={value}>{label}</button>;
60+
// Only the primary option should use the primary (blue) button style when expanded
61+
const isPrimary = primaryOptionValue && value === primaryOptionValue;
62+
return <button className={`inlined-dropdown${isPrimary ? '' : ' secondary'}`} key={value} title={label} disabled={optionDisabled || disabled} onClick={action} value={value}>{label}</button>;
6063
})
6164
:
6265
<div className='primary-split-button'>

0 commit comments

Comments
 (0)