Merge branch '301143-fix-shortcuts' into 'master'

Fix shortcuts for top nav redesign

See merge request gitlab-org/gitlab!62047

app/helpers/nav/top_nav_helper.rb

@@ -22,32 +22,26 @@ module Nav
     def build_anonymous_view_model(builder:)
       # These come from `app/views/layouts/nav/_explore.html.ham`
       if explore_nav_link?(:projects)
-        builder.add_primary_menu_item(
-          **projects_menu_item_attrs.merge(
-            {
-              active: active_nav_link?(path: %w[dashboard#show root#show projects#trending projects#starred projects#index]),
-              href: explore_root_path
-            })
+        builder.add_primary_menu_item_with_shortcut(
+          href: explore_root_path,
+          active: active_nav_link?(path: %w[dashboard#show root#show projects#trending projects#starred projects#index]),
+          **projects_menu_item_attrs
         )
       end
 
       if explore_nav_link?(:groups)
-        builder.add_primary_menu_item(
-          **groups_menu_item_attrs.merge(
-            {
-              active: active_nav_link?(controller: [:groups, 'groups/milestones', 'groups/group_members']),
-              href: explore_groups_path
-            })
+        builder.add_primary_menu_item_with_shortcut(
+          href: explore_groups_path,
+          active: active_nav_link?(controller: [:groups, 'groups/milestones', 'groups/group_members']),
+          **groups_menu_item_attrs
         )
       end
 
       if explore_nav_link?(:snippets)
-        builder.add_primary_menu_item(
-          **snippets_menu_item_attrs.merge(
-            {
-              active: active_nav_link?(controller: :snippets),
-              href: explore_snippets_path
-            })
+        builder.add_primary_menu_item_with_shortcut(
+          active: active_nav_link?(controller: :snippets),
+          href: explore_snippets_path,
+          **snippets_menu_item_attrs
         )
       end
     end
@@ -57,13 +51,13 @@ module Nav
       if dashboard_nav_link?(:projects)
         current_item = project ? current_project(project: project) : {}
 
-        builder.add_primary_menu_item(
-          **projects_menu_item_attrs.merge({
-            active: active_nav_link?(path: %w[root#index projects#trending projects#starred dashboard/projects#index]),
-            css_class: 'qa-projects-dropdown',
-            data: { track_label: "projects_dropdown", track_event: "click_dropdown", track_experiment: "new_repo" },
-            view: PROJECTS_VIEW
-          })
+        builder.add_primary_menu_item_with_shortcut(
+          active: active_nav_link?(path: %w[root#index projects#trending projects#starred dashboard/projects#index]),
+          css_class: 'qa-projects-dropdown',
+          data: { track_label: "projects_dropdown", track_event: "click_dropdown", track_experiment: "new_repo" },
+          view: PROJECTS_VIEW,
+          shortcut_href: dashboard_projects_path,
+          **projects_menu_item_attrs
         )
         builder.add_view(PROJECTS_VIEW, container_view_props(namespace: 'projects', current_item: current_item, submenu: projects_submenu))
       end
@@ -71,46 +65,47 @@ module Nav
       if dashboard_nav_link?(:groups)
         current_item = group ? current_group(group: group) : {}
 
-        builder.add_primary_menu_item(
-          **groups_menu_item_attrs.merge({
-            active: active_nav_link?(path: %w[dashboard/groups explore/groups]),
-            css_class: 'qa-groups-dropdown',
-            data: { track_label: "groups_dropdown", track_event: "click_dropdown" },
-            view: GROUPS_VIEW
-          })
+        builder.add_primary_menu_item_with_shortcut(
+          active: active_nav_link?(path: %w[dashboard/groups explore/groups]),
+          css_class: 'qa-groups-dropdown',
+          data: { track_label: "groups_dropdown", track_event: "click_dropdown" },
+          view: GROUPS_VIEW,
+          shortcut_href: dashboard_groups_path,
+          **groups_menu_item_attrs
         )
         builder.add_view(GROUPS_VIEW, container_view_props(namespace: 'groups', current_item: current_item, submenu: groups_submenu))
       end
 
       if dashboard_nav_link?(:milestones)
-        builder.add_primary_menu_item(
+        builder.add_primary_menu_item_with_shortcut(
           id: 'milestones',
           title: 'Milestones',
+          href: dashboard_milestones_path,
           active: active_nav_link?(controller: 'dashboard/milestones'),
           icon: 'clock',
           data: { qa_selector: 'milestones_link' },
-          href: dashboard_milestones_path
+          shortcut_class: 'dashboard-shortcuts-milestones'
         )
       end
 
       if dashboard_nav_link?(:snippets)
-        builder.add_primary_menu_item(
-          **snippets_menu_item_attrs.merge({
-            active: active_nav_link?(controller: 'dashboard/snippets'),
-            data: { qa_selector: 'snippets_link' },
-            href: dashboard_snippets_path
-          })
+        builder.add_primary_menu_item_with_shortcut(
+          active: active_nav_link?(controller: 'dashboard/snippets'),
+          data: { qa_selector: 'snippets_link' },
+          href: dashboard_snippets_path,
+          **snippets_menu_item_attrs
         )
       end
 
       if dashboard_nav_link?(:activity)
-        builder.add_primary_menu_item(
+        builder.add_primary_menu_item_with_shortcut(
           id: 'activity',
           title: 'Activity',
+          href: activity_dashboard_path,
           active: active_nav_link?(path: 'dashboard#activity'),
           icon: 'history',
           data: { qa_selector: 'activity_link' },
-          href: activity_dashboard_path
+          shortcut_class: 'dashboard-shortcuts-activity'
         )
       end
 
@@ -165,7 +160,8 @@ module Nav
       {
         id: 'project',
         title: _('Projects'),
-        icon: 'project'
+        icon: 'project',
+        shortcut_class: 'dashboard-shortcuts-projects'
       }
     end
 
@@ -173,7 +169,8 @@ module Nav
       {
         id: 'groups',
         title: 'Groups',
-        icon: 'group'
+        icon: 'group',
+        shortcut_class: 'dashboard-shortcuts-groups'
       }
     end
 
@@ -181,7 +178,8 @@ module Nav
       {
         id: 'snippets',
         title: _('Snippets'),
-        icon: 'snippet'
+        icon: 'snippet',
+        shortcut_class: 'dashboard-shortcuts-snippets'
       }
     end
 

app/views/layouts/nav/_top_nav.html.haml

@@ -5,3 +5,8 @@
       = sprite_icon('dot-grid', css_class: "dropdown-icon")
       = view_model[:activeTitle]
       = sprite_icon('chevron-down')
+
+.hidden
+  - view_model[:shortcuts].each do |shortcut|
+    = link_to shortcut[:href], class: shortcut[:css_class] do
+      = shortcut[:title]

lib/gitlab/nav/top_nav_view_model_builder.rb

@@ -6,9 +6,34 @@ module Gitlab
       def initialize
         @menu_builder = ::Gitlab::Nav::TopNavMenuBuilder.new
         @views = {}
+        @shortcuts = []
       end
 
-      delegate :add_primary_menu_item, :add_secondary_menu_item, to: :@menu_builder
+      # Using delegate hides the stacktrace for some errors, so we choose to be explicit.
+      # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62047#note_579031091
+      def add_primary_menu_item(**args)
+        @menu_builder.add_primary_menu_item(**args)
+      end
+
+      def add_secondary_menu_item(**args)
+        @menu_builder.add_secondary_menu_item(**args)
+      end
+
+      def add_shortcut(**args)
+        item = ::Gitlab::Nav::TopNavMenuItem.build(**args)
+
+        @shortcuts.push(item)
+      end
+
+      def add_primary_menu_item_with_shortcut(shortcut_class:, shortcut_href: nil, **args)
+        add_primary_menu_item(**args)
+        add_shortcut(
+          id: "#{args.fetch(:id)}-shortcut",
+          title: args.fetch(:title),
+          href: shortcut_href || args.fetch(:href),
+          css_class: shortcut_class
+        )
+      end
 
       def add_view(name, props)
         @views[name] = props
@@ -19,6 +44,7 @@ module Gitlab
 
         menu.merge({
           views: @views,
+          shortcuts: @shortcuts,
           activeTitle: _('Menu')
         })
       end

spec/features/dashboard/shortcuts_spec.rb

@@ -15,8 +15,6 @@ RSpec.describe 'Dashboard shortcuts', :js do
       end
 
       it 'navigate to tabs' do
-        pending_on_combined_menu_flag
-
         find('body').send_keys([:shift, 'I'])
 
         check_page_title('Issues')
@@ -49,8 +47,6 @@ RSpec.describe 'Dashboard shortcuts', :js do
       end
 
       it 'navigate to tabs' do
-        pending_on_combined_menu_flag
-
         find('body').send_keys([:shift, 'G'])
 
         find('.nothing-here-block')
@@ -74,8 +70,6 @@ RSpec.describe 'Dashboard shortcuts', :js do
   end
 
   context 'with combined_menu: feature flag on' do
-    let(:needs_rewrite_for_combined_menu_flag_on) { true }
-
     before do
       stub_feature_flags(combined_menu: true)
     end
@@ -84,16 +78,10 @@ RSpec.describe 'Dashboard shortcuts', :js do
   end
 
   context 'with combined_menu feature flag off' do
-    let(:needs_rewrite_for_combined_menu_flag_on) { false }
-
     before do
       stub_feature_flags(combined_menu: false)
     end
 
     it_behaves_like 'combined_menu: feature flag examples'
   end
-
-  def pending_on_combined_menu_flag
-    pending 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56587' if needs_rewrite_for_combined_menu_flag_on
-  end
 end

spec/helpers/nav/top_nav_helper_spec.rb

@@ -48,30 +48,51 @@ RSpec.describe Nav::TopNavHelper do
 
     context 'when current_user is nil (anonymous)' do
       it 'has expected :primary' do
-        expected_projects_item = ::Gitlab::Nav::TopNavMenuItem.build(
-          href: '/explore',
-          icon: 'project',
-          id: 'project',
-          title: 'Projects'
-        )
-        expected_groups_item = ::Gitlab::Nav::TopNavMenuItem.build(
-          href: '/explore/groups',
-          icon: 'group',
-          id: 'groups',
-          title: 'Groups'
-        )
-        expected_snippets_item = ::Gitlab::Nav::TopNavMenuItem.build(
-          href: '/explore/snippets',
-          icon: 'snippet',
-          id: 'snippets',
-          title: 'Snippets'
-        )
-        expect(subject[:primary])
-          .to eq([
-                   expected_projects_item,
-                   expected_groups_item,
-                   expected_snippets_item
-                 ])
+        expected_primary = [
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            href: '/explore',
+            icon: 'project',
+            id: 'project',
+            title: 'Projects'
+          ),
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            href: '/explore/groups',
+            icon: 'group',
+            id: 'groups',
+            title: 'Groups'
+          ),
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            href: '/explore/snippets',
+            icon: 'snippet',
+            id: 'snippets',
+            title: 'Snippets'
+          )
+        ]
+        expect(subject[:primary]).to eq(expected_primary)
+      end
+
+      it 'has expected :shortcuts' do
+        expected_shortcuts = [
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            href: '/explore',
+            id: 'project-shortcut',
+            title: 'Projects',
+            css_class: 'dashboard-shortcuts-projects'
+          ),
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            href: '/explore/groups',
+            id: 'groups-shortcut',
+            title: 'Groups',
+            css_class: 'dashboard-shortcuts-groups'
+          ),
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            href: '/explore/snippets',
+            id: 'snippets-shortcut',
+            title: 'Snippets',
+            css_class: 'dashboard-shortcuts-snippets'
+          )
+        ]
+        expect(subject[:shortcuts]).to eq(expected_shortcuts)
       end
     end
 
@@ -82,6 +103,7 @@ RSpec.describe Nav::TopNavHelper do
         expect(subject).to eq({ activeTitle: active_title,
                                 primary: [],
                                 secondary: [],
+                                shortcuts: [],
                                 views: {} })
       end
 
@@ -105,6 +127,16 @@ RSpec.describe Nav::TopNavHelper do
           expect(subject[:primary]).to eq([expected_primary])
         end
 
+        it 'has expected :shortcuts' do
+          expected_shortcuts = ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'project-shortcut',
+            title: 'Projects',
+            href: '/dashboard/projects',
+            css_class: 'dashboard-shortcuts-projects'
+          )
+          expect(subject[:shortcuts]).to eq([expected_shortcuts])
+        end
+
         context 'projects' do
           it 'has expected :currentUserName' do
             expect(projects_view[:currentUserName]).to eq(current_user.username)
@@ -191,6 +223,16 @@ RSpec.describe Nav::TopNavHelper do
           expect(subject[:primary]).to eq([expected_primary])
         end
 
+        it 'has expected :shortcuts' do
+          expected_shortcuts = ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'groups-shortcut',
+            title: 'Groups',
+            href: '/dashboard/groups',
+            css_class: 'dashboard-shortcuts-groups'
+          )
+          expect(subject[:shortcuts]).to eq([expected_shortcuts])
+        end
+
         context 'groups' do
           it 'has expected :currentUserName' do
             expect(groups_view[:currentUserName]).to eq(current_user.username)
@@ -268,6 +310,16 @@ RSpec.describe Nav::TopNavHelper do
           )
           expect(subject[:primary]).to eq([expected_primary])
         end
+
+        it 'has expected :shortcuts' do
+          expected_shortcuts = ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'milestones-shortcut',
+            title: 'Milestones',
+            href: '/dashboard/milestones',
+            css_class: 'dashboard-shortcuts-milestones'
+          )
+          expect(subject[:shortcuts]).to eq([expected_shortcuts])
+        end
       end
 
       context 'with snippets' do
@@ -285,6 +337,16 @@ RSpec.describe Nav::TopNavHelper do
           )
           expect(subject[:primary]).to eq([expected_primary])
         end
+
+        it 'has expected :shortcuts' do
+          expected_shortcuts = ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'snippets-shortcut',
+            title: 'Snippets',
+            href: '/dashboard/snippets',
+            css_class: 'dashboard-shortcuts-snippets'
+          )
+          expect(subject[:shortcuts]).to eq([expected_shortcuts])
+        end
       end
 
       context 'with activity' do
@@ -302,6 +364,16 @@ RSpec.describe Nav::TopNavHelper do
           )
           expect(subject[:primary]).to eq([expected_primary])
         end
+
+        it 'has expected :shortcuts' do
+          expected_shortcuts = ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'activity-shortcut',
+            title: 'Activity',
+            href: '/dashboard/activity',
+            css_class: 'dashboard-shortcuts-activity'
+          )
+          expect(subject[:shortcuts]).to eq([expected_shortcuts])
+        end
       end
 
       context 'when sherlock is enabled' do