Merge branch '52366-improved-group-lists-ui' into 'master'

Resolve "Improved group lists UI"

Closes #52366

See merge request gitlab-org/gitlab-ce!26542

12 files changed, 114 insertions, 115 deletions

diff --git a/app/assets/javascripts/groups/components/group_item.vue b/app/assets/javascripts/groups/components/group_item.vue
index d5130cd331d..9909f437fc8 100644
--- a/app/assets/javascripts/groups/components/group_item.vue
+++ b/app/assets/javascripts/groups/components/group_item.vue
@@ -1,12 +1,15 @@
 <script>
+import { GlLoadingIcon } from '@gitlab/ui';
 import { visitUrl } from '../../lib/utils/url_utility';
 import tooltip from '../../vue_shared/directives/tooltip';
 import identicon from '../../vue_shared/components/identicon.vue';
 import eventHub from '../event_hub';
+import { VISIBILITY_TYPE_ICON, GROUP_VISIBILITY_TYPE } from '../constants';
 
 import itemCaret from './item_caret.vue';
 import itemTypeIcon from './item_type_icon.vue';
 import itemStats from './item_stats.vue';
+import itemStatsValue from './item_stats_value.vue';
 import itemActions from './item_actions.vue';
 
 export default {
@@ -14,10 +17,12 @@ export default {
     tooltip,
   },
   components: {
+    GlLoadingIcon,
     identicon,
     itemCaret,
     itemTypeIcon,
     itemStats,
+    itemStatsValue,
     itemActions,
   },
   props: {
@@ -57,6 +62,12 @@ export default {
     isGroup() {
       return this.group.type === 'group';
     },
+    visibilityIcon() {
+      return VISIBILITY_TYPE_ICON[this.group.visibility];
+    },
+    visibilityTooltip() {
+      return GROUP_VISIBILITY_TYPE[this.group.visibility];
+    },
   },
   methods: {
     onClickRowGroup(e) {
@@ -80,43 +91,61 @@ export default {
   <li :id="groupDomId" :class="rowClass" class="group-row" @click.stop="onClickRowGroup">
     <div
       :class="{ 'project-row-contents': !isGroup }"
-      class="group-row-contents d-flex justify-content-end align-items-center"
+      class="group-row-contents d-flex align-items-center"
     >
       <div class="folder-toggle-wrap append-right-4 d-flex align-items-center">
         <item-caret :is-group-open="group.isOpen" />
         <item-type-icon :item-type="group.type" :is-group-open="group.isOpen" />
       </div>
+      <gl-loading-icon
+        v-if="group.isChildrenLoading"
+        size="md"
+        class="d-none d-sm-inline-flex flex-shrink-0 append-right-10"
+      />
       <div
-        :class="{ 'content-loading': group.isChildrenLoading }"
-        class="avatar-container rect-avatar s24 d-none d-sm-flex"
+        :class="{ 'd-sm-flex': !group.isChildrenLoading }"
+        class="avatar-container rect-avatar s32 d-none flex-grow-0 flex-shrink-0 "
       >
         <a :href="group.relativePath" class="no-expand">
-          <img v-if="hasAvatar" :src="group.avatarUrl" class="avatar s24" />
-          <identicon v-else :entity-id="group.id" :entity-name="group.name" size-class="s24" />
+          <img v-if="hasAvatar" :src="group.avatarUrl" class="avatar s32" />
+          <identicon v-else :entity-id="group.id" :entity-name="group.name" size-class="s32" />
         </a>
       </div>
-      <div class="group-text flex-grow">
-        <div class="title namespace-title append-right-8">
-          <a
-            v-tooltip
-            :href="group.relativePath"
-            :title="group.fullName"
-            class="no-expand"
-            data-placement="bottom"
-            >{{
-              // ending bracket must be by closing tag to prevent
-              // link hover text-decoration from over-extending
-              group.name
-            }}</a
-          >
-          <span v-if="group.permission" class="user-access-role"> {{ group.permission }} </span>
+      <div class="group-text-container d-flex flex-fill align-items-center">
+        <div class="group-text flex-grow-1 flex-shrink-1">
+          <div class="d-flex align-items-center flex-wrap title namespace-title append-right-8">
+            <a
+              v-tooltip
+              :href="group.relativePath"
+              :title="group.fullName"
+              class="no-expand prepend-top-8 append-right-8"
+              data-placement="bottom"
+              >{{
+                // ending bracket must be by closing tag to prevent
+                // link hover text-decoration from over-extending
+                group.name
+              }}</a
+            >
+            <item-stats-value
+              :icon-name="visibilityIcon"
+              :title="visibilityTooltip"
+              css-class="item-visibility d-inline-flex align-items-center prepend-top-8 append-right-4"
+            />
+            <span v-if="group.permission" class="user-access-role prepend-top-8">
+              {{ group.permission }}
+            </span>
+          </div>
+          <div v-if="group.description" class="description">
+            <span v-html="group.description"> </span>
+          </div>
         </div>
-        <div v-if="group.description" class="description">
-          <span v-html="group.description"> </span>
+        <div
+          class="metadata align-items-md-center d-flex flex-grow-1 flex-shrink-0 flex-wrap justify-content-md-between"
+        >
+          <item-actions v-if="isGroup" :group="group" :parent-group="parentGroup" />
+          <item-stats :item="group" class="group-stats prepend-top-2 d-none d-md-flex" />
         </div>
       </div>
-      <item-stats :item="group" class="group-stats prepend-top-2" />
-      <item-actions v-if="isGroup" :group="group" :parent-group="parentGroup" />
     </div>
     <group-folder
       v-if="group.isOpen && hasChildren"
diff --git a/app/assets/javascripts/groups/components/item_actions.vue b/app/assets/javascripts/groups/components/item_actions.vue
index a7995865c77..cafd22731b1 100644
--- a/app/assets/javascripts/groups/components/item_actions.vue
+++ b/app/assets/javascripts/groups/components/item_actions.vue
@@ -44,31 +44,31 @@ export default {
 </script>
 
 <template>
-  <div class="controls">
+  <div class="controls d-flex justify-content-end">
     <a
-      v-if="group.canEdit"
+      v-if="group.canLeave"
       v-tooltip
-      :href="group.editPath"
-      :title="editBtnTitle"
-      :aria-label="editBtnTitle"
+      :href="group.leavePath"
+      :title="leaveBtnTitle"
+      :aria-label="leaveBtnTitle"
       data-container="body"
       data-placement="bottom"
-      class="edit-group btn no-expand"
+      class="leave-group btn btn-xs no-expand"
+      @click.prevent="onLeaveGroup"
     >
-      <icon name="settings" />
+      <icon name="leave" css-classes="position-top-0" />
     </a>
     <a
-      v-if="group.canLeave"
+      v-if="group.canEdit"
       v-tooltip
-      :href="group.leavePath"
-      :title="leaveBtnTitle"
-      :aria-label="leaveBtnTitle"
+      :href="group.editPath"
+      :title="editBtnTitle"
+      :aria-label="editBtnTitle"
       data-container="body"
       data-placement="bottom"
-      class="leave-group btn no-expand"
-      @click.prevent="onLeaveGroup"
+      class="edit-group btn btn-xs no-expand"
     >
-      <icon name="leave" />
+      <icon name="settings" css-classes="position-top-0" />
     </a>
   </div>
 </template>
diff --git a/app/assets/javascripts/groups/components/item_caret.vue b/app/assets/javascripts/groups/components/item_caret.vue
index 43b9607ea8e..18ea4819878 100644
--- a/app/assets/javascripts/groups/components/item_caret.vue
+++ b/app/assets/javascripts/groups/components/item_caret.vue
@@ -21,5 +21,5 @@ export default {
 </script>
 
 <template>
-  <span class="folder-caret"> <icon :size="12" :name="iconClass" /> </span>
+  <span class="folder-caret append-right-4"> <icon :size="10" :name="iconClass" /> </span>
 </template>
diff --git a/app/assets/javascripts/groups/components/item_stats.vue b/app/assets/javascripts/groups/components/item_stats.vue
index bc6851ea2bf..734a9a89c72 100644
--- a/app/assets/javascripts/groups/components/item_stats.vue
+++ b/app/assets/javascripts/groups/components/item_stats.vue
@@ -48,7 +48,7 @@ export default {
       :title="__('Subgroups')"
       :value="item.subgroupCount"
       css-class="number-subgroups"
-      icon-name="folder"
+      icon-name="folder-o"
     />
     <item-stats-value
       v-if="isGroup"
@@ -70,12 +70,6 @@ export default {
       css-class="project-stars"
       icon-name="star"
     />
-    <item-stats-value
-      :icon-name="visibilityIcon"
-      :title="visibilityTooltip"
-      css-class="item-visibility"
-      tooltip-placement="left"
-    />
     <div v-if="isProject" class="last-updated">
       <time-ago-tooltip :time="item.updatedAt" tooltip-placement="bottom" />
     </div>
diff --git a/app/assets/javascripts/groups/components/item_type_icon.vue b/app/assets/javascripts/groups/components/item_type_icon.vue
index e1ebd03cb5f..ae69fbd7bde 100644
--- a/app/assets/javascripts/groups/components/item_type_icon.vue
+++ b/app/assets/javascripts/groups/components/item_type_icon.vue
@@ -20,7 +20,7 @@ export default {
   computed: {
     iconClass() {
       if (this.itemType === ITEM_TYPE.GROUP) {
-        return this.isGroupOpen ? 'folder-open' : 'folder';
+        return this.isGroupOpen ? 'folder-open' : 'folder-o';
       }
       return 'bookmark';
     },
diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss
index 555a3fe0dc7..954551fef97 100644
--- a/app/assets/stylesheets/framework/lists.scss
+++ b/app/assets/stylesheets/framework/lists.scss
@@ -133,7 +133,6 @@ ul.content-list {
 
     .description {
       @include str-truncated;
-      color: $gl-text-color-secondary;
     }
 
     .controls {
diff --git a/app/assets/stylesheets/framework/mixins.scss b/app/assets/stylesheets/framework/mixins.scss
index ad5096761cd..bf0f1da6aa3 100644
--- a/app/assets/stylesheets/framework/mixins.scss
+++ b/app/assets/stylesheets/framework/mixins.scss
@@ -65,7 +65,7 @@
   .stats {
     float: right;
     line-height: $list-text-height;
-    color: $gl-text-color;
+    color: $gl-text-color-secondary;
 
     span {
       margin-right: 15px;
diff --git a/app/assets/stylesheets/pages/groups.scss b/app/assets/stylesheets/pages/groups.scss
index 656202f4e58..cff2e274390 100644
--- a/app/assets/stylesheets/pages/groups.scss
+++ b/app/assets/stylesheets/pages/groups.scss
@@ -168,12 +168,6 @@
   }
 }
 
-.groups-listing {
-  .group-list-tree .group-row:first-child {
-    border-top: 0;
-  }
-}
-
 .card {
   .shared_runners_limit_under_quota {
     color: $green-500;
@@ -260,7 +254,6 @@ table.pipeline-project-metrics tr td {
   color: $gl-text-color-secondary;
   font-size: 12px;
   line-height: 20px;
-  margin: -5px 3px;
   padding: 0 $label-padding;
   border: 1px solid $border-color;
   border-radius: $label-border-radius;
@@ -294,39 +287,6 @@ table.pipeline-project-metrics tr td {
 }
 
 .group-list-tree {
-  .avatar-container.content-loading {
-    position: relative;
-
-    > a,
-    > a .avatar {
-      height: 100%;
-      border-radius: 50%;
-    }
-
-    > a {
-      padding: 2px;
-
-      .avatar {
-        border: 2px solid $white-normal;
-
-        &.identicon {
-          line-height: 15px;
-        }
-      }
-    }
-
-    &::after {
-      content: '';
-      position: absolute;
-      height: 100%;
-      width: 100%;
-      background-color: transparent;
-      border: 2px outset $gl-gray-200;
-      border-radius: 50%;
-      animation: spin-avatar 3s infinite linear;
-    }
-  }
-
   .folder-toggle-wrap {
     font-size: 0;
     flex-shrink: 0;
@@ -339,13 +299,14 @@ table.pipeline-project-metrics tr td {
   .folder-caret,
   .item-type-icon {
     display: inline-block;
+    color: $gl-text-color-secondary;
   }
 
   .folder-caret {
-    width: 15px;
+    width: $gl-font-size-large;
 
     svg {
-      margin-bottom: 1px;
+      margin-bottom: 2px;
     }
   }
 
@@ -420,7 +381,7 @@ table.pipeline-project-metrics tr td {
   }
 
   .group-row-contents {
-    padding: $gl-padding-top;
+    padding: $gl-padding;
 
     &:hover {
       border-color: $blue-200;
@@ -428,10 +389,15 @@ table.pipeline-project-metrics tr td {
       cursor: pointer;
     }
 
+    .group-text-container,
     .group-text {
       min-width: 0; // allows for truncated text within flex children
     }
 
+    .group-text {
+      flex-basis: 100%;
+    }
+
     .avatar-container {
       flex-shrink: 0;
 
@@ -441,6 +407,21 @@ table.pipeline-project-metrics tr td {
       }
     }
 
+    .title {
+      margin-top: -$gl-padding-8; // negative margin required for flex-wrap
+      font-size: $gl-font-size-large;
+    }
+
+    .item-visibility {
+      color: $gl-text-color-secondary;
+    }
+
+    @include media-breakpoint-down(md) {
+      .title {
+        font-size: $gl-font-size;
+      }
+    }
+
     &.has-more-items {
       display: block;
       padding: 20px 10px;
@@ -477,17 +458,18 @@ table.pipeline-project-metrics tr td {
     }
 
     .controls {
-      flex-shrink: 0;
+      flex-basis: 90px;
 
       > .btn {
-        margin: 0 0 0 $btn-margin-5;
+        margin: 0 $btn-side-margin 0 0;
+        color: $gl-text-color-secondary;
       }
     }
-  }
 
-  @include media-breakpoint-down(xs) {
-    .group-stats {
-      display: none;
+    .metadata {
+      @include media-breakpoint-up(md) {
+        flex-basis: 240px;
+      }
     }
   }
 
diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss
index 151af843c95..c80beceae52 100644
--- a/app/assets/stylesheets/pages/projects.scss
+++ b/app/assets/stylesheets/pages/projects.scss
@@ -889,7 +889,6 @@ pre.light-well {
     @include basic-list-stats;
     display: flex;
     align-items: center;
-    color: $gl-text-color-secondary;
     padding: $gl-padding 0;
 
     @include media-breakpoint-up(lg) {
@@ -952,10 +951,6 @@ pre.light-well {
 
     .description {
       line-height: 1.5;
-
-      @include media-breakpoint-up(md) {
-        color: $gl-text-color;
-      }
     }
 
     @include media-breakpoint-down(md) {
diff --git a/changelogs/unreleased/52366-improved-group-lists-ui.yml b/changelogs/unreleased/52366-improved-group-lists-ui.yml
new file mode 100644
index 00000000000..99e5b67a56c
--- /dev/null
+++ b/changelogs/unreleased/52366-improved-group-lists-ui.yml
@@ -0,0 +1,5 @@
+---
+title: Improve group list UI
+merge_request: 26542
+author:
+type: changed
diff --git a/spec/javascripts/groups/components/group_item_spec.js b/spec/javascripts/groups/components/group_item_spec.js
index 4d6d0c895b6..cc88a7ac6c1 100644
--- a/spec/javascripts/groups/components/group_item_spec.js
+++ b/spec/javascripts/groups/components/group_item_spec.js
@@ -156,6 +156,8 @@ describe('GroupItemComponent', () => {
 
   describe('template', () => {
     it('should render component template correctly', () => {
+      const visibilityIconEl = vm.$el.querySelector('.item-visibility');
+
       expect(vm.$el.getAttribute('id')).toBe('group-55');
       expect(vm.$el.classList.contains('group-row')).toBeTruthy();
 
@@ -173,6 +175,11 @@ describe('GroupItemComponent', () => {
 
       expect(vm.$el.querySelector('.title')).toBeDefined();
       expect(vm.$el.querySelector('.title a.no-expand')).toBeDefined();
+
+      expect(visibilityIconEl).not.toBe(null);
+      expect(visibilityIconEl.dataset.originalTitle).toBe(vm.visibilityTooltip);
+      expect(visibilityIconEl.querySelectorAll('svg').length).toBeGreaterThan(0);
+
       expect(vm.$el.querySelector('.access-type')).toBeDefined();
       expect(vm.$el.querySelector('.description')).toBeDefined();
 
diff --git a/spec/javascripts/groups/components/item_stats_spec.js b/spec/javascripts/groups/components/item_stats_spec.js
index 00d6a4817d7..b2441babf3f 100644
--- a/spec/javascripts/groups/components/item_stats_spec.js
+++ b/spec/javascripts/groups/components/item_stats_spec.js
@@ -108,18 +108,6 @@ describe('ItemStatsComponent', () => {
       vm.$destroy();
     });
 
-    it('renders item visibility icon and tooltip correctly', () => {
-      const vm = createComponent();
-
-      const visibilityIconEl = vm.$el.querySelector('.item-visibility');
-
-      expect(visibilityIconEl).not.toBe(null);
-      expect(visibilityIconEl.dataset.originalTitle).toBe(vm.visibilityTooltip);
-      expect(visibilityIconEl.querySelectorAll('svg').length).toBeGreaterThan(0);
-
-      vm.$destroy();
-    });
-
     it('renders start count and last updated information for project item correctly', () => {
       const item = Object.assign({}, mockParentGroupItem, {
         type: ITEM_TYPE.PROJECT,