Merge branch 'fix-awards' into 'master'

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

See merge request gitlab-org/gitlab!62621

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;