Skip to content

Ensure re-request-review command is handled in activityBarViewProvider - #4540

Merged
Alex Ross (alexr00) merged 1 commit into
microsoft:mainfrom
joshuaobrien:ensure-re-request-review-case-handled-in-activity-bar
Feb 20, 2023
Merged

Ensure re-request-review command is handled in activityBarViewProvider#4540
Alex Ross (alexr00) merged 1 commit into
microsoft:mainfrom
joshuaobrien:ensure-re-request-review-case-handled-in-activity-bar

Conversation

@joshuaobrien

@joshuaobrien Joshua O'Brien (joshuaobrien) commented Feb 19, 2023

Copy link
Copy Markdown
Contributor

Whilst working on #4539 I noticed that this command wasn't handled for this view. This change reveals a bit of layout shift after the user clicks the button. I'm happy to take a look at that this week :)

Before

old-click.mp4

After

new-click.mp4

}
}

private reRequestReview(message: IRequestMessage<string>): void {

@joshuaobrien Joshua O'Brien (joshuaobrien) Feb 19, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copied verbatim from

private reRequestReview(message: IRequestMessage<string>): void {

Would you like me to have a look at unifying any duplicated handlers?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's likely a lot of duplication between these two files. I'll take it as a debt item to clean them up.

@alexr00 Alex Ross (alexr00) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the fix!

}
}

private reRequestReview(message: IRequestMessage<string>): void {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's likely a lot of duplication between these two files. I'll take it as a debt item to clean them up.

@alexr00 Alex Ross (alexr00) added this to the February 2023 milestone Feb 20, 2023
@alexr00
Alex Ross (alexr00) enabled auto-merge (squash) February 20, 2023 15:26
@alexr00
Alex Ross (alexr00) merged commit 1cad0cf into microsoft:main Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants