Merge branch 'dm-consistent-note-paddings' into 'master'

More consistent padding of notes, system notes, and note form

See merge request !11720

8 files changed, 75 insertions, 54 deletions

diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index b0b1cfd6c8a..59c52c1e497 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -1212,7 +1212,7 @@ const normalizeNewlines = function(str) {
         `<li id="${uniqueId}" class="note being-posted fade-in-half timeline-entry">
            <div class="timeline-entry-inner">
               <div class="timeline-icon">
-                 <a href="/${currentUsername}"><span class="dummy-avatar"></span></a>
+                 <a href="/${currentUsername}"><span class="avatar dummy-avatar"></span></a>
               </div>
               <div class="timeline-content ${discussionClass}">
                  <div class="note-header">
diff --git a/app/assets/stylesheets/framework/flash.scss b/app/assets/stylesheets/framework/flash.scss
index eadb9409fee..25b4feca3c3 100644
--- a/app/assets/stylesheets/framework/flash.scss
+++ b/app/assets/stylesheets/framework/flash.scss
@@ -36,6 +36,10 @@
       border-radius: 0;
     }
   }
+
+  &:empty {
+    margin: 0;
+  }
 }
 
 @media (max-width: $screen-sm-max) {
diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss
index 0c3407f34f8..785b09e622f 100644
--- a/app/assets/stylesheets/framework/typography.scss
+++ b/app/assets/stylesheets/framework/typography.scss
@@ -21,6 +21,10 @@
     margin-top: 0;
   }
 
+  > :last-child {
+    margin-bottom: 0;
+  }
+
   // Single code lines should wrap
   code {
     font-family: $monospace_font;
@@ -157,7 +161,7 @@
   ul,
   ol {
     padding: 0;
-    margin: 0 0 16px !important;
+    margin: 0 0 16px;
   }
 
   ul:dir(rtl),
diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss
index d79ae47f589..c2346f2f1c3 100644
--- a/app/assets/stylesheets/pages/issuable.scss
+++ b/app/assets/stylesheets/pages/issuable.scss
@@ -431,7 +431,7 @@
 }
 
 .detail-page-description {
-  padding: 16px 0 0;
+  padding: 16px 0;
 
   small {
     color: $gray-darkest;
@@ -441,7 +441,7 @@
 .edited-text {
   color: $gray-darkest;
   display: block;
-  margin: 0 0 16px;
+  margin: 16px 0 0;
 
   .author_link {
     color: $gray-darkest;
diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss
index 49e453c7dbc..875e47cdff3 100644
--- a/app/assets/stylesheets/pages/note_form.scss
+++ b/app/assets/stylesheets/pages/note_form.scss
@@ -28,7 +28,7 @@
 .note-edit-form {
   .note-form-actions {
     position: relative;
-    margin: $gl-padding 0;
+    margin: $gl-padding 0 0;
   }
 
   .note-preview-holder {
@@ -124,10 +124,18 @@
 }
 
 .discussion-form {
-  padding: $gl-padding-top $gl-padding;
+  padding: $gl-padding-top $gl-padding $gl-padding;
   background-color: $white-light;
 }
 
+.discussion-notes .disabled-comment {
+  padding: 6px 0;
+}
+
+.notes-form > li {
+  border: 0;
+}
+
 .note-edit-form {
   display: none;
   font-size: 14px;
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index a78c2576e0f..32d2e9ba4bd 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -14,19 +14,6 @@ ul.notes {
   margin: 0;
   padding: 0;
 
-  .timeline-icon {
-    float: left;
-
-    svg {
-      width: 16px;
-      height: 16px;
-      fill: $gray-darkest;
-      position: absolute;
-      left: 0;
-      top: 16px;
-    }
-  }
-
   .timeline-content {
     margin-left: 55px;
 
@@ -56,21 +43,22 @@ ul.notes {
     position: relative;
   }
 
-  .note {
-    padding: $gl-padding $gl-btn-padding 0;
+  > li {
+    padding: $gl-padding $gl-btn-padding;
     display: block;
     position: relative;
     border-bottom: 1px solid $white-normal;
 
+    &:last-child {
+      // Override `.timeline > li:last-child { border-bottom: none; }`
+      border-bottom: 1px solid $white-normal;
+    }
+
     &.being-posted {
       pointer-events: none;
       opacity: 0.5;
 
       .dummy-avatar {
-        display: inline-block;
-        height: 40px;
-        width: 40px;
-        border-radius: 50%;
         background-color: $kdb-border;
         border: 1px solid darken($kdb-border, 25%);
       }
@@ -126,7 +114,7 @@ ul.notes {
 
     .note-awards {
       .js-awards-block {
-        margin-bottom: 16px;
+        margin-top: 16px;
       }
     }
 
@@ -161,7 +149,7 @@ ul.notes {
 
   .system-note {
     font-size: 14px;
-    padding: 0;
+    padding-left: 0;
     clear: both;
 
     @media (min-width: $screen-sm-min) {
@@ -198,11 +186,22 @@ ul.notes {
       }
     }
 
-    .timeline-content {
-      padding: 14px 10px;
+    .timeline-icon {
+      float: left;
 
+      svg {
+        width: 16px;
+        height: 16px;
+        fill: $gray-darkest;
+        position: absolute;
+        left: 0;
+        top: 2px;
+      }
+    }
+
+    .timeline-content {
       @media (min-width: $screen-sm-min) {
-        margin-left: 20px;
+        margin-left: 30px;
       }
     }
 
@@ -596,10 +595,15 @@ ul.notes {
 .discussion-body,
 .diff-file {
   .notes .note {
-    padding: 10px 15px;
+    padding-left: $gl-padding;
+    padding-right: $gl-padding;
 
     &.system-note {
-      padding: 0;
+      padding-left: 0;
+
+      @media (min-width: $screen-sm-min) {
+        margin-left: 70px;
+      }
     }
   }
 }
@@ -613,17 +617,11 @@ ul.notes {
 }
 
 .disabled-comment {
-  margin-left: -$gl-padding-top;
-  margin-right: -$gl-padding-top;
   background-color: $gray-light;
   border-radius: $border-radius-base;
   border: 1px solid $border-gray-normal;
   color: $note-disabled-comment-color;
-  line-height: 200px;
-
-  .disabled-comment-text {
-    line-height: normal;
-  }
+  padding: 90px 0;
 
   a {
     color: $gl-link-color;
diff --git a/app/views/discussions/_notes.html.haml b/app/views/discussions/_notes.html.haml
index 7ba3f3f6c42..db5ab939948 100644
--- a/app/views/discussions/_notes.html.haml
+++ b/app/views/discussions/_notes.html.haml
@@ -1,10 +1,11 @@
 .discussion-notes
   %ul.notes{ data: { discussion_id: discussion.id } }
     = render partial: "shared/notes/note", collection: discussion.notes, as: :note
-    .flash-container
 
-  - if current_user
-    .discussion-reply-holder
+  .flash-container
+
+  .discussion-reply-holder
+    - if can_create_note?
       - if discussion.potentially_resolvable?
         - line_type = local_assigns.fetch(:line_type, nil)
 
@@ -19,3 +20,10 @@
             = render "discussions/jump_to_next", discussion: discussion
       - else
         = link_to_reply_discussion(discussion)
+    - elsif !current_user
+      .disabled-comment.text-center
+        Please
+        = link_to "register", new_session_path(:user, redirect_to_referer: 'yes')
+        or
+        = link_to "sign in", new_session_path(:user, redirect_to_referer: 'yes')
+        to reply
diff --git a/app/views/shared/notes/_notes_with_form.html.haml b/app/views/shared/notes/_notes_with_form.html.haml
index 785b1b22a49..5902798dfd0 100644
--- a/app/views/shared/notes/_notes_with_form.html.haml
+++ b/app/views/shared/notes/_notes_with_form.html.haml
@@ -3,24 +3,23 @@
 
 = render 'shared/notes/edit_form', project: @project
 
-%ul.notes.notes-form.timeline
-  %li.timeline-entry
-    .flash-container.timeline-content
+- if can_create_note?
+  %ul.notes.notes-form.timeline
+    %li.timeline-entry
+      .flash-container.timeline-content
 
-    - if  can_create_note?
       .timeline-icon.hidden-xs.hidden-sm
         %a.author_link{ href: user_path(current_user) }
           = image_tag avatar_icon(current_user), alt: current_user.to_reference, class: 'avatar s40'
       .timeline-content.timeline-content-form
         = render "shared/notes/form", view: diff_view
-    - elsif !current_user
-      .disabled-comment.text-center
-        .disabled-comment-text.inline
-          Please
-          = link_to "register", new_session_path(:user, redirect_to_referer: 'yes')
-          or
-          = link_to "sign in", new_session_path(:user, redirect_to_referer: 'yes')
-          to post a comment
+- elsif !current_user
+  .disabled-comment.text-center.prepend-top-default
+    Please
+    = link_to "register", new_session_path(:user, redirect_to_referer: 'yes')
+    or
+    = link_to "sign in", new_session_path(:user, redirect_to_referer: 'yes')
+    to comment
 
 :javascript
   var notes = new Notes("#{notes_url}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{diff_view}", #{autocomplete})