Merge branch...

Merge branch '346003-box-shadow-for-gl-link-items-in-the-nav-should-be-outline-instead' into 'master'

Box shadow should be applied to non gl-link links in side nav

See merge request gitlab-org/gitlab!76144

app/assets/stylesheets/framework/contextual_sidebar.scss

@@ -376,6 +376,18 @@
       }
     }
   }
+
+  li > a.gl-link {
+    // undo gl-link text items for things in the sidebar - including sub menus
+    // defined in https://gitlab.com/gitlab-org/gitlab-ui/-/blob/5431e0ca5149d4e02e3d5d617d194ac9609bb82d/src/components/base/link/link.scss
+    @include gl-text-body;
+
+    &:active,
+    &:focus,
+    &:focus:active {
+      @include gl-text-decoration-none;
+    }
+  }
 }
 
 .sidebar-sub-level-items {

app/assets/stylesheets/startup/startup-dark.scss

@@ -422,6 +422,19 @@ h1 {
   width: 32px;
   height: 32px;
 }
+.gl-link {
+  font-size: 0.875rem;
+  color: #428fdc;
+}
+.gl-link:active {
+  color: #9dc7f1;
+}
+.gl-link:active {
+  text-decoration: underline;
+  box-shadow: 0 0 0 1px rgba(51, 51, 51, 0.4),
+    0 0 0 4px rgba(66, 143, 220, 0.48);
+  outline: none;
+}
 .gl-button {
   display: inline-flex;
 }
@@ -1372,6 +1385,12 @@ input {
   font-weight: 400;
   color: #9dc7f1;
 }
+.sidebar-top-level-items li > a.gl-link {
+  color: #fafafa;
+}
+.sidebar-top-level-items li > a.gl-link:active {
+  text-decoration: none;
+}
 .sidebar-sub-level-items {
   padding-top: 0;
   padding-bottom: 0;

app/assets/stylesheets/startup/startup-general.scss

@@ -403,6 +403,19 @@ h1 {
   width: 32px;
   height: 32px;
 }
+.gl-link {
+  font-size: 0.875rem;
+  color: #1f75cb;
+}
+.gl-link:active {
+  color: #0b5cad;
+}
+.gl-link:active {
+  text-decoration: underline;
+  box-shadow: 0 0 0 1px rgba(255, 255, 255, 0.4),
+    0 0 0 4px rgba(31, 117, 203, 0.48);
+  outline: none;
+}
 .gl-button {
   display: inline-flex;
 }
@@ -1353,6 +1366,12 @@ input {
   font-weight: 400;
   color: #0b5cad;
 }
+.sidebar-top-level-items li > a.gl-link {
+  color: #303030;
+}
+.sidebar-top-level-items li > a.gl-link:active {
+  text-decoration: none;
+}
 .sidebar-sub-level-items {
   padding-top: 0;
   padding-bottom: 0;

app/views/groups/_invite_members_side_nav_link.html.haml

 .js-invite-members-trigger{ data: { trigger_source: 'group-side-nav',
-  classes: 'gl-text-decoration-none! gl-shadow-none! gl-text-body!',
   icon: 'users',
   display_text: title,
   trigger_element: 'side-nav'} }

app/views/projects/_invite_members_side_nav_link.html.haml

 .js-invite-members-trigger{ data: { trigger_source: 'project-side-nav',
-  classes: 'gl-text-decoration-none! gl-shadow-none! gl-text-body!',
   icon: 'users',
   display_text: title,
   trigger_element: 'side-nav'} }

app/views/shared/nav/_scope_menu.html.haml

 = nav_link(**scope_menu.active_routes, html_options: scope_menu.nav_link_html_options) do
-  = link_to scope_menu.link, **scope_menu.container_html_options, data: { qa_selector: 'sidebar_menu_link', qa_menu_item: scope_qa_menu_item(scope_menu.container) } do
+  = link_to scope_menu.link, **scope_menu.link_html_options, data: { qa_selector: 'sidebar_menu_link', qa_menu_item: scope_qa_menu_item(scope_menu.container) } do
     %span{ class: scope_avatar_classes(scope_menu.container) }
       = source_icon(scope_menu.container, alt: scope_menu.title, class: ['avatar', 'avatar-tile', 's32'], width: 32, height: 32)
     %span.sidebar-context-title

app/views/shared/nav/_sidebar_hidden_menu_item.html.haml

 %li.hidden
-  = link_to sidebar_hidden_menu_item.link, **sidebar_hidden_menu_item.container_html_options do
+  = link_to sidebar_hidden_menu_item.link, **sidebar_hidden_menu_item.link_html_options do
     = sidebar_hidden_menu_item.title

app/views/shared/nav/_sidebar_menu.html.haml

@@ -2,7 +2,7 @@
   - if sidebar_menu.menu_with_partial?
     = render_if_exists sidebar_menu.menu_partial, **sidebar_menu.menu_partial_options
   - else
-    = link_to sidebar_menu.link, **sidebar_menu.container_html_options, data: { qa_selector: 'sidebar_menu_link', qa_menu_item: sidebar_menu.title } do
+    = link_to sidebar_menu.link, **sidebar_menu.link_html_options, data: { qa_selector: 'sidebar_menu_link', qa_menu_item: sidebar_menu.title } do
       - if sidebar_menu.icon_or_image?
         %span.nav-icon-container
           - if sidebar_menu.image_path

app/views/shared/nav/_sidebar_menu_item.html.haml

 = nav_link(**sidebar_menu_item.active_routes, html_options: sidebar_menu_item.nav_link_html_options) do
-  = link_to sidebar_menu_item.link, **sidebar_menu_item.container_html_options, data: { qa_selector: 'sidebar_menu_item_link', qa_menu_item: sidebar_menu_item.title } do
+  = link_to sidebar_menu_item.link, **sidebar_menu_item.link_html_options, data: { qa_selector: 'sidebar_menu_item_link', qa_menu_item: sidebar_menu_item.title } do
     %span
       = sidebar_menu_item.title
       - if sidebar_menu_item.sprite_icon

ee/app/assets/javascripts/contextual_sidebar/components/trial_status_widget.vue

@@ -43,13 +43,7 @@ export default {
 </script>
 
 <template>
-  <gl-link
-    :id="containerId"
-    :title="widgetTitle"
-    :href="plansHref"
-    class="gl-text-decoration-none! gl-text-body!"
-    @click="onWidgetClick"
-  >
+  <gl-link :id="containerId" :title="widgetTitle" :href="plansHref" @click="onWidgetClick">
     <div class="gl-display-flex gl-flex-direction-column gl-align-items-stretch gl-w-full">
       <span class="gl-display-flex gl-align-items-center">
         <span class="nav-icon-container svg-container">

ee/app/assets/stylesheets/startup/startup-dark.scss

@@ -422,6 +422,19 @@ h1 {
   width: 32px;
   height: 32px;
 }
+.gl-link {
+  font-size: 0.875rem;
+  color: #428fdc;
+}
+.gl-link:active {
+  color: #9dc7f1;
+}
+.gl-link:active {
+  text-decoration: underline;
+  box-shadow: 0 0 0 1px rgba(51, 51, 51, 0.4),
+    0 0 0 4px rgba(66, 143, 220, 0.48);
+  outline: none;
+}
 .gl-button {
   display: inline-flex;
 }
@@ -1372,6 +1385,12 @@ input {
   font-weight: 400;
   color: #9dc7f1;
 }
+.sidebar-top-level-items li > a.gl-link {
+  color: #fafafa;
+}
+.sidebar-top-level-items li > a.gl-link:active {
+  text-decoration: none;
+}
 .sidebar-sub-level-items {
   padding-top: 0;
   padding-bottom: 0;

ee/app/assets/stylesheets/startup/startup-general.scss

@@ -403,6 +403,19 @@ h1 {
   width: 32px;
   height: 32px;
 }
+.gl-link {
+  font-size: 0.875rem;
+  color: #1f75cb;
+}
+.gl-link:active {
+  color: #0b5cad;
+}
+.gl-link:active {
+  text-decoration: underline;
+  box-shadow: 0 0 0 1px rgba(255, 255, 255, 0.4),
+    0 0 0 4px rgba(31, 117, 203, 0.48);
+  outline: none;
+}
 .gl-button {
   display: inline-flex;
 }
@@ -1353,6 +1366,12 @@ input {
   font-weight: 400;
   color: #0b5cad;
 }
+.sidebar-top-level-items li > a.gl-link {
+  color: #303030;
+}
+.sidebar-top-level-items li > a.gl-link:active {
+  text-decoration: none;
+}
 .sidebar-sub-level-items {
   padding-top: 0;
   padding-bottom: 0;

ee/spec/frontend/contextual_sidebar/__snapshots__/trial_status_widget_spec.js.snap

@@ -2,7 +2,6 @@
 
 exports[`TrialStatusWidget component without the optional containerId prop matches the snapshot 1`] = `
 <gl-link-stub
-  class="gl-text-decoration-none! gl-text-body!"
   href="billing/path-for/group"
   title="Ultimate Trial – 20 days left"
 >

lib/sidebars/concerns/container_with_html_options.rb

@@ -3,6 +3,8 @@
 module Sidebars
   module Concerns
     module ContainerWithHtmlOptions
+      include LinkWithHtmlOptions
+
       # The attributes returned from this method
       # will be applied to helper methods like
       # `link_to` or the div containing the container.

lib/sidebars/concerns/link_with_html_options.rb

+# frozen_string_literal: true
+
+module Sidebars
+  module Concerns
+    module LinkWithHtmlOptions
+      # add on specific items as the pertain to `link_to` objects specifically
+      def link_html_options
+        container_html_options.tap do |html_options|
+          html_options[:class] = [*html_options[:class], 'gl-link'].join(' ')
+        end
+      end
+    end
+  end
+end

lib/sidebars/menu_item.rb

@@ -2,6 +2,8 @@
 
 module Sidebars
   class MenuItem
+    include ::Sidebars::Concerns::LinkWithHtmlOptions
+
     attr_reader :title, :link, :active_routes, :item_id, :container_html_options, :sprite_icon, :sprite_icon_html_options, :hint_html_options
 
     def initialize(title:, link:, active_routes:, item_id: nil, container_html_options: {}, sprite_icon: nil, sprite_icon_html_options: {}, hint_html_options: {})

spec/lib/sidebars/concerns/link_with_html_options_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Concerns::LinkWithHtmlOptions do
+  let(:options) { {} }
+
+  subject { Class.new { include Sidebars::Concerns::LinkWithHtmlOptions }.new }
+
+  before do
+    allow(subject).to receive(:container_html_options).and_return(options)
+  end
+
+  describe '#link_html_options' do
+    context 'with existing classes' do
+      let(:options) do
+        {
+          class: '_class1_ _class2_',
+          aria: { label: '_label_' }
+        }
+      end
+
+      it 'includes class and default aria-label attribute' do
+        result = {
+          class: '_class1_ _class2_ gl-link',
+          aria: { label: '_label_' }
+        }
+
+        expect(subject.link_html_options).to eq(result)
+      end
+    end
+
+    context 'without existing classes' do
+      it 'includes gl-link class' do
+        expect(subject.link_html_options).to eq(class: 'gl-link')
+      end
+    end
+  end
+end