Adding Edit/Delete functions to inline comments #1701
Conversation
|
|
||
| <StackPanel Orientation="Horizontal" HorizontalAlignment="Right" DockPanel.Dock="Right" | ||
| Visibility="{Binding CanEditOrDelete, Converter={ui:BooleanToVisibilityConverter}}"> | ||
| <ui:GitHubActionLink Content="Edit" |
There was a problem hiding this comment.
Rather than Edit, should this be Update comment like on .com? 👆
There was a problem hiding this comment.
I think this references the Edit / Delete links in the screenshot above?
If so, I think it's okay to leave this the way how it is. However, I'm 👍 for having the submit button text be "Update comment" like the dotcom screenshot you've posted.
| </Style.Triggers> | ||
| </Style> | ||
| </StackPanel.Style> | ||
| <Button Command="{Binding CommitEdit}">Edit</Button> |
There was a problem hiding this comment.
Per @jcansdale's comment, can we change this to "Update comment" so it's inline with the interaction on dotcom?
| public ReactiveCommand<ICommentModel> EditComment | ||
| { | ||
| get { return editComment; } | ||
| set |
There was a problem hiding this comment.
I see you followed PostComment and made this setter public - I think that was an oversight, it should be protected. When you fix this, could you fix PostComment too?
There was a problem hiding this comment.
Made them protected
| public ReactiveCommand<object> DeleteComment | ||
| { | ||
| get { return deleteComment; } | ||
| set |
StanleyGoldman
force-pushed
the
features/inline-comment-edit-and-delete
branch
from
bd0e9a9 to
cc12bea
Compare
|
@meaghanlewis good catch. I fixed that up. |
| x => x.EditState, | ||
| x => x == CommentEditState.None && user.Login.Equals(currentUser.Login)); | ||
|
|
||
| canDelete.Subscribe(b => CanDelete = b); |
There was a problem hiding this comment.
You could use ObservableAsPropertyHelper to make this a bit neater.
|
Oops, clicked "approve" before writing my comment ;) Looks good and works well - just one small suggestion, but I don't think that should hold up an approval. |
There was a problem hiding this comment.
Oops, spoke too soon. This fails to edit a pending comment:
To repro:
- Add a comment, click start review
- Try to edit the comment
This is because you're using the REST API for editing the comment and the REST API doesn't work with pending comments. We'll have to use GraphQL for this.
| /// <param name="inReplyTo">The REST ID of the comment to reply to.</param> | ||
| /// <param name="inReplyToNodeId">The GraphQL ID of the comment to reply to.</param> | ||
| /// <returns></returns> | ||
| /// <returns>A comment model.</returns> |
There was a problem hiding this comment.
Ah. this is a test.
|
Nice catch @grokys! I'm able to update comments now when a review is pending. |
|
@StanleyGoldman - on a similar note about pending reviews...i don't seem to be able to delete comments while a review is pending. And then trying to update after clicking delete shows a
|
|
One more thought about this. On dotcom when I go to delete a comment, I get a confirmation popup from the browser:
I'm wondering if it would be possible to add a confirmation in Visual Studio as well? |
| CommentEditState state; | ||
| DateTimeOffset updatedAt; | ||
| string undoBody; | ||
| private bool canDelete; |
|
@meaghanlewis I opened #1713 to reflect your comments |
Mainly merging #1701 Conflicts: src/GitHub.Exports.Reactive/Services/IPullRequestSession.cs src/GitHub.InlineReviews/SampleData/CommentThreadViewModelDesigner.cs src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs src/GitHub.InlineReviews/Services/PullRequestSession.cs src/GitHub.InlineReviews/Services/PullRequestSessionService.cs src/GitHub.InlineReviews/ViewModels/CommentThreadViewModel.cs src/GitHub.InlineReviews/ViewModels/CommentViewModel.cs src/GitHub.InlineReviews/ViewModels/ICommentThreadViewModel.cs test/GitHub.InlineReviews.UnitTests/ViewModels/PullRequestReviewCommentViewModelTests.cs
6 participants
Depends on: