Merge pull request #14100 from hellcp-work/diff-component

Use the comment components for rendering comments
openSUSE · Apr 3, 2023 · c1bc9c7 · c1bc9c7
2 parents 75e6940 + 803cf62
commit c1bc9c7
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 43 deletions.
diff --git a/src/api/app/assets/javascripts/webui/comment.js b/src/api/app/assets/javascripts/webui/comment.js
@@ -21,11 +21,11 @@ function validateForm(e) {
 
 $(document).ready(function(){
   // Disable submit button if textarea is empty and enable otherwise
-  $('.comments-list,.comment_new,.timeline').on('keyup', '.comment-field', function(e) {
+  $('.comments-list,.comment_new,.timeline,.diff').on('keyup', '.comment-field', function(e) {
     validateForm(e);
   });
 
-  $('.comments-list,.comment_new,.timeline').on('keyup click', '.comment-field', function() {
+  $('.comments-list,.comment_new,.timeline,.diff').on('keyup click', '.comment-field', function() {
     resizeTextarea(this);
   });
 
@@ -39,7 +39,7 @@ $(document).ready(function(){
   });
 
   // This is being used to render only the comment thread for a reply by the beta request show view
-  $('.timeline').on('ajax:complete', '.post-comment-form', function(_, data) {
+  $('.timeline,.diff').on('ajax:complete', '.post-comment-form', function(_, data) {
     $(this).closest('.comments-thread').html(data.responseText);
   });
 
@@ -63,7 +63,7 @@ $(document).ready(function(){
   });
 
   // This is being used to update the comment with the updated content after an edit from the beta request show view
-  $('.timeline').on('ajax:complete', '.put-comment-form', function(_, data) {
+  $('.timeline,.diff').on('ajax:complete', '.put-comment-form', function(_, data) {
     $(this).closest('.comments-thread').html(data.responseText);
   });
 
@@ -82,10 +82,10 @@ $(document).ready(function(){
   });
 
   // This is used to delete comments from the beta request show view, we are not gonna get an updated comment thread like
-  $('.timeline').on('ajax:complete', '.delete-comment-form', function(_, data) {
-    var $this = $(this),
-        $commentsList = $this.closest('.comments-thread'),
-        $form = $('#delete-comment-modal-' + $this.data('commentId'));
+  $('body').on('ajax:complete', '#delete-comment-modal form', function(_, data) {
+    var $commentId = $(this).attr('action').split('/').slice(-1),
+        $commentsList = $('[name=comment-' + $commentId + ']').closest('.comments-thread'),
+        $form = $('#delete-comment-modal');
 
     $form.modal('hide');
     // We have to wait until the modal is hidden to properly remove the dialog UI

diff --git a/src/api/app/assets/stylesheets/webui/diff.scss b/src/api/app/assets/stylesheets/webui/diff.scss
@@ -35,9 +35,23 @@
         margin-left: calc(2 * (var(--line-index-digits) + 2rem + #{$card-border-width}) + 0.5rem + #{$card-border-width});
         margin-top: -1.75rem;
       }
-      .line-comment hr:first-of-type {
+      .line-comment .fa-comment {
         display: none;
       }
+      .line-comment .timeline-item-comment {
+        @extend .ms-4;
+      }
+      .line-comment .comments-thread {
+        &:first-child {
+          @extend .border-top;
+          > :first-child { @extend .mt-3; }
+        }
+        &:last-child {
+          @extend .border-bottom;
+          > :last-child { @extend .mb-3; }
+        }
+        @extend .px-3;
+      }
     }
     .line {
       font-family: $font-family-monospace;

diff --git a/src/api/app/components/bs_request_comment_component.html.haml b/src/api/app/components/bs_request_comment_component.html.haml
@@ -18,8 +18,11 @@
                       class: 'dropdown-item collapsed') do
               Edit
           - if policy(comment).destroy?
-            = link_to('#', data: { 'bs-toggle': 'modal', 'bs-target': "#delete-comment-modal-#{comment.id}" },
-                      class: 'dropdown-item delete_link', title: 'Delete comment') do
+            = link_to('#', data: { 'bs-toggle': 'modal',
+                                   'bs-target': '#delete-comment-modal',
+                                   confirmation_text: 'Please confirm deletion of comment',
+                                   action: comment_path(comment) },
+                           class: 'dropdown-item delete_link', title: 'Delete comment') do
               Delete
 
     = helpers.render_as_markdown(comment)
@@ -29,9 +32,6 @@
         = render(partial: 'webui/comment/comment_field', locals: { form_method: :put,
          comment: comment, commentable: commentable, element_suffix: "edit_#{comment.id}", has_cancel: true })
 
-    - if policy(comment).destroy?
-      = render(partial: 'webui/comment/delete_dialog', locals: { comment: comment })
-
   -# This should be refactored to avoid relying on global state
   - if User.session # rubocop:disable ViewComponent/AvoidGlobalState
     .mb-2

diff --git a/src/api/app/components/diff_component.html.haml b/src/api/app/components/diff_component.html.haml
@@ -23,12 +23,11 @@
         - if commentable
           .line-comment{ id: "comment#{id}" }
             - if commented_lines.include?(id)
-              .p-3.border-top.border-bottom
-                .comments-list
-                  = render(partial: 'webui/comment/comment_list',
-                           locals: { comment: Comment.new, commentable: commentable, diff_ref: id })
+              - commentable.comments.where(diff_ref: id).each do |comment|
+                .comments-thread.p-3
+                  = render BsRequestCommentComponent.new(comment: comment, level: 1, commentable: commentable)
             - else
-              .comments-list
+              .diff-comments
                 -# rubocop:disable ViewComponent/AvoidGlobalState
                 - if User.session
                   = link_to(request_inline_comment_path(commentable.bs_request, commentable, id),

diff --git a/src/api/app/controllers/webui/comments_controller.rb b/src/api/app/controllers/webui/comments_controller.rb
@@ -20,7 +20,7 @@ def create
                :unprocessable_entity
              end
 
-    if Flipper.enabled?(:request_show_redesign, User.session) && comment.commentable_type == 'BsRequest'
+    if Flipper.enabled?(:request_show_redesign, User.session) && ['BsRequest', 'BsRequestAction'].include?(comment.commentable_type)
       render(partial: 'webui/comment/beta/comments_thread',
              locals: { comment: comment.root, commentable: @commentable, level: 1 },
              status: status)
@@ -47,7 +47,7 @@ def update
 
     respond_to do |format|
       format.html do
-        if Flipper.enabled?(:request_show_redesign, User.session) && comment.commentable_type == 'BsRequest'
+        if Flipper.enabled?(:request_show_redesign, User.session) && ['BsRequest', 'BsRequestAction'].include?(comment.commentable_type)
           render(partial: 'webui/comment/beta/comments_thread',
                  locals: { comment: comment.root, commentable: comment.commentable, level: 1 },
                  status: status)
@@ -77,7 +77,7 @@ def destroy
                :unprocessable_entity
              end
 
-    if Flipper.enabled?(:request_show_redesign, User.session) && comment.commentable_type == 'BsRequest'
+    if Flipper.enabled?(:request_show_redesign, User.session) && ['BsRequest', 'BsRequestAction'].include?(comment.commentable_type)
       # if we're a root comment with no replies there is no need to re-render anything
       return head(:ok) if comment.root? && comment.leaf?
 

diff --git a/src/api/app/views/webui/comment/_comment_list.html.haml b/src/api/app/views/webui/comment/_comment_list.html.haml
@@ -1,18 +1,8 @@
-- opts = defined?(diff_ref) ? { diff_ref: } : {}
-- comments = commentable.comments.where(opts).without_parent.includes(:user)
-- comments.each do |comment|
+- commentable.comments.without_parent.includes(:user).each do |comment|
   = render partial: 'webui/comment/content', locals: { comment: comment, commentable: commentable, level: 1 }
 
 .comment_new.mt-3
-  - if opts[:diff_ref] && comments.present?
-    %a{ href: "#new_comment_expander_#{diff_ref}",
-        data: { 'bs-toggle': 'collapse', 'bs-target': "#new_comment_expander_#{diff_ref}" },
-        role: 'button', aria: { expanded: 'false' } }
-      %input.expander.form-control{ placeholder: 'Write new comment...', type: 'text' }
-    .collapse{ id: "new_comment_expander_#{diff_ref}" }
-      = render partial: 'webui/comment/new', locals: opts.merge({ commentable: commentable })
-  - else
-    = render partial: 'webui/comment/new', locals: opts.merge({ commentable: commentable })
+  = render partial: 'webui/comment/new', locals: { commentable: commentable }
 
 :javascript
   $(document).ready(function() {

diff --git a/src/api/app/views/webui/comment/_new.html.haml b/src/api/app/views/webui/comment/_new.html.haml
@@ -1,7 +1,6 @@
 - if User.session
-  - opts = defined?(diff_ref) ? { line: diff_ref, element_suffix: "new_comment_#{diff_ref}", has_cancel: true } : {}
   = render(partial: 'webui/comment/comment_field',
-    locals: { form_method: :post, comment: Comment.new, commentable: commentable, element_suffix: 'new_comment', has_cancel: false }.merge(opts))
+    locals: { form_method: :post, comment: Comment.new, commentable: commentable, element_suffix: 'new_comment', has_cancel: false })
 - else
   .alert.alert-warning{ role: 'alert' }
     Login required, please

diff --git a/src/api/app/views/webui/request/_inline_comment.html.haml b/src/api/app/views/webui/request/_inline_comment.html.haml
@@ -1,4 +1,8 @@
-.p-3.border-top.border-bottom
-  .comments-list
-    = render(partial: 'webui/comment/comment_list',
-      locals: { comment: Comment.new, commentable:, diff_ref: })
+.comments-list
+  - commentable.comments.where(diff_ref:).each do |comment|
+    .comments-thread
+      = render BsRequestCommentComponent.new(comment: comment, level: 1, commentable: commentable)
+  .comments-thread
+    = render(partial: 'webui/comment/comment_field',
+    locals: { form_method: :post, comment: Comment.new, commentable: commentable,
+              line: diff_ref, element_suffix: "new_comment_#{diff_ref}", has_cancel: true })
diff --git a/src/api/app/views/webui/request/beta_show.html.haml b/src/api/app/views/webui/request/beta_show.html.haml
@@ -131,6 +131,10 @@
         .tab-pane.fade.p-2#mentioned-issues{ 'aria-labelledby': 'mentioned-issues-tab', role: 'tabpanel' }
           = render partial: 'webui/request/beta_show_tabs/mentioned_issues', locals: { issues: @issues }
 
+  = render DeleteConfirmationDialogComponent.new(modal_id: 'delete-comment-modal',
+                                                 method: :delete,
+                                                 options: { modal_title: 'Delete comment?', remote: true })
+
 - content_for :ready_function do
   :plain
     var anchor = document.location.hash;

diff --git a/src/api/app/views/webui/request/inline_comment.js.erb b/src/api/app/views/webui/request/inline_comment.js.erb
@@ -1,6 +1,6 @@
 $('#flash').empty();
-$('#comment<%= @line %> .comments-list').html("<%= escape_javascript(render(partial: 'webui/request/inline_comment', locals: { comment: @comment, commentable: @active_action, diff_ref: @line })) %>");
-$("#comment<%= @line %> .comments-list textarea").focus();
+$('#comment<%= @line %> .diff-comments').html("<%= escape_javascript(render(partial: 'webui/request/inline_comment', locals: { comment: @comment, commentable: @active_action, diff_ref: @line })) %>");
+$("#comment<%= @line %> .diff-comments textarea").focus();
 $("#comment<%= @line %>").on('click', '.cancel-comment', function (e) {
-  $('#comment<%= @line %> .comments-list').html("<%= escape_javascript(render(partial: 'webui/request/add_inline_comment', locals: { commentable: @active_action, diff_ref: @line })) %>");
+  $('#comment<%= @line %> .diff-comments').html("<%= escape_javascript(render(partial: 'webui/request/add_inline_comment', locals: { commentable: @active_action, diff_ref: @line })) %>");
 });