Improve reviewing PR UX (#19612)

Changes

7 changed files (+74/-3)

• routers/web/repo/pull.go
  • @@ -752,11 +752,27 @@ func ViewPullFiles(ctx *context.Context) {
    if ctx.Written() { return } - ctx.Data["CurrentReview"], err = models.GetCurrentReview(ctx.Doer, issue) + + currentReview, err := models.GetCurrentReview(ctx.Doer, issue) if err != nil && !models.IsErrReviewNotExist(err) { ctx.ServerError("GetCurrentReview", err) return } + numPendingCodeComments := int64(0) + if currentReview != nil { + numPendingCodeComments, err = models.CountComments(&models.FindCommentsOptions{ + Type: models.CommentTypeCode, + ReviewID: currentReview.ID, + IssueID: issue.ID, + }) + if err != nil { + ctx.ServerError("CountComments", err) + return + } + } + ctx.Data["CurrentReview"] = currentReview + ctx.Data["PendingCodeCommentNumber"] = numPendingCodeComments + getBranchData(ctx, issue) ctx.Data["IsIssuePoster"] = ctx.IsSigned && issue.IsPoster(ctx.Doer.ID) ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)
• templates/repo/diff/comments.tmpl
  • @@ -37,7 +37,7 @@
    <div class="comment-header-right actions df ac"> {{if and .Review}} {{if eq .Review.Type 0}} - <div class="ui label basic small yellow"> + <div class="ui label basic small yellow pending-label"> {{$.root.i18n.Tr "repo.issues.review.pending"}} </div> {{else}}
• templates/repo/diff/new_review.tmpl
  • @@ -1,6 +1,7 @@
    <div class="ui top right pointing dropdown custom" id="review-box"> <div class="ui tiny green button btn-review"> {{.i18n.Tr "repo.diff.review"}} + <span class="ui small label review-comments-counter" data-pending-comment-number="{{.PendingCodeCommentNumber}}">{{.PendingCodeCommentNumber}}</span> {{svg "octicon-triangle-down" 14 "dropdown icon"}} </div> <div class="menu review-box">
• web_src/js/features/repo-diff.js
  • @@ -6,8 +6,23 @@ import {validateTextareaNonEmpty} from './comp/EasyMDE.js';
    const {csrfToken} = window.config; export function initRepoDiffReviewButton() { + const $reviewBox = $('#review-box'); + const $counter = $reviewBox.find('.review-comments-counter'); + $(document).on('click', 'button[name="is_review"]', (e) => { - $(e.target).closest('form').append('<input type="hidden" name="is_review" value="true">'); + const $form = $(e.target).closest('form'); + $form.append('<input type="hidden" name="is_review" value="true">'); + + // Watch for the form's submit event. + $form.on('submit', () => { + const num = parseInt($counter.attr('data-pending-comment-number')) + 1 || 1; + $counter.attr('data-pending-comment-number', num); + $counter.text(num); + // Force the browser to reflow the DOM. This is to ensure that the browser replay the animation + $reviewBox.removeClass('pulse'); + $reviewBox.width(); + $reviewBox.addClass('pulse'); + }); }); }
• web_src/js/features/repo-issue.js
  • @@ -160,6 +160,16 @@ export function initRepoIssueCommentDelete() {
    _csrf: csrfToken, }).done(() => { const $conversationHolder = $this.closest('.conversation-holder'); + + // Check if this was a pending comment. + if ($conversationHolder.find('.pending-label').length) { + const $counter = $('#review-box .review-comments-counter'); + let num = parseInt($counter.attr('data-pending-comment-number')) - 1 || 0; + num = Math.max(num, 0); + $counter.attr('data-pending-comment-number', num); + $counter.text(num); + } + $(`#${$this.data('comment-id')}`).remove(); if ($conversationHolder.length && !$conversationHolder.find('.comment').length) { const path = $conversationHolder.data('path');
• web_src/less/_review.less
  • @@ -242,6 +242,19 @@ a.blob-excerpt:hover {
    border: none !important; } + #review-box .review-comments-counter { + background-color: var(--color-primary-light-4); + color: #fff; + } + + #review-box:hover .review-comments-counter { + background-color: var(--color-primary-light-5); + } + + #review-box .review-comments-counter[data-pending-comment-number="0"] { + display: none; + } + .pull.files.diff [id] { scroll-margin-top: 99px;
• web_src/less/animations.less
  • @@ -50,3 +50,19 @@
    opacity: 0; } } + + @keyframes pulse { + 0% { + transform: scale(1); + } + 50% { + transform: scale(1.8); + } + 100% { + transform: scale(1); + } + } + + .pulse { + animation: pulse 2s linear; + }