Fix layout, spacing, and style of reaction emoji on issuables

Standardizes spacing between emojis, button categories and
selected states, emoji button height, and surrounding
button layout for better responsive behavior.

Changelog: fixed

app/assets/javascripts/emoji/awards_app/index.js

@@ -32,7 +32,7 @@ export default (el) => {
           canAwardEmoji: this.canAwardEmoji,
           currentUserId: this.currentUserId,
           defaultAwards: ['thumbsup', 'thumbsdown'],
-          selectedClass: 'gl-bg-blue-50! is-active',
+          selectedClass: 'selected',
         },
         on: {
           award: this.toggleAward,

app/assets/javascripts/emoji/components/picker.vue

@@ -85,7 +85,7 @@ export default {
       :boundary="getBoundaryElement()"
       :class="dropdownClass"
       menu-class="dropdown-extended-height"
-      category="tertiary"
+      category="secondary"
       no-flip
       right
       lazy

app/assets/javascripts/notes/components/note_actions.vue

@@ -304,7 +304,7 @@ export default {
         v-else
         v-gl-tooltip
         :class="{ 'js-user-authored': isAuthoredByCurrentUser }"
-        class="note-action-button note-emoji-button add-reaction-button btn-icon js-add-award js-note-emoji"
+        class="note-action-button note-emoji-button add-reaction-button js-add-award js-note-emoji"
         category="tertiary"
         variant="default"
         :title="$options.i18n.addReactionLabel"

app/assets/javascripts/vue_shared/components/awards_list.vue

@@ -173,7 +173,7 @@ export default {
       v-for="awardList in groupedAwards"
       :key="awardList.name"
       v-gl-tooltip.viewport
-      class="gl-mr-3"
+      class="gl-mr-3 gl-my-2"
       :class="awardList.classes"
       :title="awardList.title"
       data-testid="award-button"
@@ -184,10 +184,10 @@ export default {
       </template>
       <span class="js-counter">{{ awardList.list.length }}</span>
     </gl-button>
-    <div v-if="canAwardEmoji" class="award-menu-holder">
+    <div v-if="canAwardEmoji" class="award-menu-holder gl-my-2">
       <emoji-picker
         v-if="glFeatures.improvedEmojiPicker"
-        :toggle-class="['add-reaction-button gl-relative!', { 'is-active': isMenuOpen }]"
+        :toggle-class="['add-reaction-button btn-icon gl-relative!', { 'is-active': isMenuOpen }]"
         @click="handleAward"
         @shown="setIsMenuOpen(true)"
         @hidden="setIsMenuOpen(false)"

app/assets/stylesheets/framework/awards.scss

@@ -255,27 +255,9 @@
   // This forces the height and width of the inner content to match
   // other gl-buttons despite all child elements being set to
   // `position:absolute`
-  &::after {
-    content: '\a0';
-    display: block !important;
-    width: 1em;
-    color: transparent;
-  }
-
-  .reaction-control-icon {
-    position: absolute;
-    top: 0;
-    left: 0;
-    height: 100%;
-    width: 100%;
-
-    // center the icon vertically and horizontally within the button
-    display: flex;
-    align-items: center;
-    justify-content: center;
 
-    @include transition(opacity, transform);
 
+  .reaction-control-icon {
     .gl-icon {
       height: $default-icon-size;
       width: $default-icon-size;
@@ -283,32 +265,26 @@
   }
 
   .reaction-control-icon-neutral {
-    opacity: 1;
+    display: flex;
   }
 
   .reaction-control-icon-positive,
   .reaction-control-icon-super-positive {
-    opacity: 0;
+    display: none;
   }
 
   &:hover,
   &.active,
   &:active,
   &.is-active {
-    // extra specificty added to override another selector
-    .reaction-control-icon .gl-icon {
-      color: $blue-500;
-      transform: scale(1.15);
-    }
-
     .reaction-control-icon-neutral {
-      opacity: 0;
+      display: none;
     }
   }
 
   &:hover {
     .reaction-control-icon-positive {
-      opacity: 1;
+      display: flex;
     }
   }
 
@@ -316,11 +292,11 @@
   &:active,
   &.is-active {
     .reaction-control-icon-positive {
-      opacity: 0;
+      display: none;
     }
 
     .reaction-control-icon-super-positive {
-      opacity: 1;
+      display: flex;
     }
   }
 
@@ -336,17 +312,13 @@
       }
 
       .reaction-control-icon-neutral {
-        opacity: 1;
+        display: flex;
       }
 
       .reaction-control-icon-positive,
       .reaction-control-icon-super-positive {
-        opacity: 0;
+        display: none;
       }
     }
   }
 }
-
-.awards .is-active {
-  box-shadow: inset 0 0 0 1px $blue-200;
-}

app/assets/stylesheets/framework/blocks.scss

@@ -72,7 +72,7 @@
   }
 
   &.content-component-block {
-    padding: 11px 0;
+    padding: 8px 0;
     background-color: $body-bg;
   }
 
@@ -253,7 +253,7 @@
 }
 
 .content-block-small {
-  padding: 10px 0;
+  padding: 4px 0;
 }
 
 .landing {

app/assets/stylesheets/pages/issuable.scss

@@ -67,10 +67,6 @@
 
   .emoji-block {
     padding: $gl-padding-4 0;
-
-    @include media-breakpoint-down(md) {
-      padding: $gl-padding-8 0;
-    }
   }
 }
 

app/assets/stylesheets/pages/issues.scss

@@ -130,10 +130,6 @@ ul.related-merge-requests > li {
     &:not(:only-child) {
       margin-right: $gl-padding-8;
     }
-
-    @include media-breakpoint-down(md) {
-      margin-top: $gl-padding-8;
-    }
   }
 }
 
@@ -154,10 +150,6 @@ ul.related-merge-requests > li {
 
   .btn-group:not(.hidden) {
     display: flex;
-
-    @include media-breakpoint-down(md) {
-      margin-top: $gl-padding-8;
-    }
   }
 
   .js-create-merge-request {

app/views/projects/merge_requests/_awards_block.html.haml

-.content-block.content-block-small.emoji-list-container.js-noteable-awards
+.content-block.emoji-block.emoji-list-container.js-noteable-awards
   = render 'award_emoji/awards_block', awardable: @merge_request, inline: true, api_awards_path: award_emoji_merge_request_api_path(@merge_request) do
-    .ml-auto.mt-auto.mb-auto
+    .ml-auto.gl-my-2
       #js-vue-sort-issue-discussions
       = render "projects/merge_requests/discussion_filter"

app/views/shared/issue_type/_emoji_block.html.haml

@@ -4,7 +4,7 @@
   .row.gl-m-0.gl-justify-content-space-between
     .js-noteable-awards
       = render 'award_emoji/awards_block', awardable: issuable, inline: true, api_awards_path: api_awards_path
-    .new-branch-col
+    .new-branch-col.gl-my-2
       = render_if_exists "projects/issues/timeline_toggle", issuable: issuable
       #js-vue-sort-issue-discussions
       #js-vue-discussion-filter{ data: { default_filter: current_user&.notes_filter_for(issuable), notes_filters: UserPreference.notes_filters.to_json } }

spec/features/issues/user_interacts_with_awards_spec.rb

@@ -62,11 +62,11 @@ RSpec.describe 'User interacts with awards' do
 
       page.within('.awards') do
         expect(page).to have_selector('[data-testid="award-button"]')
-        expect(page.find('[data-testid="award-button"].is-active .js-counter')).to have_content('1')
-        expect(page).to have_css('[data-testid="award-button"].is-active[title="You"]')
+        expect(page.find('[data-testid="award-button"].selected .js-counter')).to have_content('1')
+        expect(page).to have_css('[data-testid="award-button"].selected[title="You"]')
 
         expect do
-          page.find('[data-testid="award-button"].is-active').click
+          page.find('[data-testid="award-button"].selected').click
           wait_for_requests
         end.to change { page.all('[data-testid="award-button"]').size }.from(3).to(2)
       end
@@ -205,7 +205,7 @@ RSpec.describe 'User interacts with awards' do
       it 'adds award to issue' do
         first('[data-testid="award-button"]').click
 
-        expect(page).to have_selector('[data-testid="award-button"].is-active')
+        expect(page).to have_selector('[data-testid="award-button"].selected')
         expect(first('[data-testid="award-button"]')).to have_content '1'
 
         visit project_issue_path(project, issue)
@@ -215,7 +215,7 @@ RSpec.describe 'User interacts with awards' do
 
       it 'removes award from issue' do
         first('[data-testid="award-button"]').click
-        find('[data-testid="award-button"].is-active').click
+        find('[data-testid="award-button"].selected').click
 
         expect(first('[data-testid="award-button"]')).to have_content '0'
 

spec/features/merge_request/user_awards_emoji_spec.rb

@@ -18,7 +18,7 @@ RSpec.describe 'Merge request > User awards emoji', :js do
 
     it 'adds award to merge request' do
       first('[data-testid="award-button"]').click
-      expect(page).to have_selector('[data-testid="award-button"].is-active')
+      expect(page).to have_selector('[data-testid="award-button"].selected')
       expect(first('[data-testid="award-button"]')).to have_content '1'
 
       visit project_merge_request_path(project, merge_request)
@@ -27,7 +27,7 @@ RSpec.describe 'Merge request > User awards emoji', :js do
 
     it 'removes award from merge request' do
       first('[data-testid="award-button"]').click
-      find('[data-testid="award-button"].is-active').click
+      find('[data-testid="award-button"].selected').click
       expect(first('[data-testid="award-button"]')).to have_content '0'
 
       visit project_merge_request_path(project, merge_request)

spec/frontend/vue_shared/components/__snapshots__/awards_list_spec.js.snap

@@ -5,7 +5,7 @@ exports[`vue_shared/components/awards_list default matches snapshot 1`] = `
   class="awards js-awards-block"
 >
   <button
-    class="btn gl-mr-3 btn-default btn-md gl-button"
+    class="btn gl-mr-3 gl-my-2 btn-default btn-md gl-button"
     data-testid="award-button"
     title="Ada, Leonardo, and Marie"
     type="button"
@@ -35,7 +35,7 @@ exports[`vue_shared/components/awards_list default matches snapshot 1`] = `
     </span>
   </button>
   <button
-    class="btn gl-mr-3 btn-default btn-md gl-button selected"
+    class="btn gl-mr-3 gl-my-2 btn-default btn-md gl-button selected"
     data-testid="award-button"
     title="You, Ada, and Marie"
     type="button"
@@ -65,7 +65,7 @@ exports[`vue_shared/components/awards_list default matches snapshot 1`] = `
     </span>
   </button>
   <button
-    class="btn gl-mr-3 btn-default btn-md gl-button"
+    class="btn gl-mr-3 gl-my-2 btn-default btn-md gl-button"
     data-testid="award-button"
     title="Ada and Jane"
     type="button"
@@ -95,7 +95,7 @@ exports[`vue_shared/components/awards_list default matches snapshot 1`] = `
     </span>
   </button>
   <button
-    class="btn gl-mr-3 btn-default btn-md gl-button selected"
+    class="btn gl-mr-3 gl-my-2 btn-default btn-md gl-button selected"
     data-testid="award-button"
     title="You, Ada, Jane, and Leonardo"
     type="button"
@@ -125,7 +125,7 @@ exports[`vue_shared/components/awards_list default matches snapshot 1`] = `
     </span>
   </button>
   <button
-    class="btn gl-mr-3 btn-default btn-md gl-button selected"
+    class="btn gl-mr-3 gl-my-2 btn-default btn-md gl-button selected"
     data-testid="award-button"
     title="You"
     type="button"
@@ -155,7 +155,7 @@ exports[`vue_shared/components/awards_list default matches snapshot 1`] = `
     </span>
   </button>
   <button
-    class="btn gl-mr-3 btn-default btn-md gl-button"
+    class="btn gl-mr-3 gl-my-2 btn-default btn-md gl-button"
     data-testid="award-button"
     title="Marie"
     type="button"
@@ -185,7 +185,7 @@ exports[`vue_shared/components/awards_list default matches snapshot 1`] = `
     </span>
   </button>
   <button
-    class="btn gl-mr-3 btn-default btn-md gl-button selected"
+    class="btn gl-mr-3 gl-my-2 btn-default btn-md gl-button selected"
     data-testid="award-button"
     title="You"
     type="button"
@@ -216,7 +216,7 @@ exports[`vue_shared/components/awards_list default matches snapshot 1`] = `
   </button>
    
   <div
-    class="award-menu-holder"
+    class="award-menu-holder gl-my-2"
   >
     <button
       aria-label="Add reaction"

spec/frontend/vue_shared/components/awards_list_spec.js

@@ -41,7 +41,14 @@ const TEST_AWARDS = [
 ];
 const TEST_ADD_BUTTON_CLASS = 'js-test-add-button-class';
 
-const REACTION_CONTROL_CLASSES = ['btn', 'gl-mr-3', 'btn-default', 'btn-md', 'gl-button'];
+const REACTION_CONTROL_CLASSES = [
+  'btn',
+  'gl-mr-3',
+  'gl-my-2',
+  'btn-default',
+  'btn-md',
+  'gl-button',
+];
 
 describe('vue_shared/components/awards_list', () => {
   let wrapper;