Minor review fixes

Externalize strings

Simplify locals passed through views

Ensure we can still filter when logged out

Cleanup conditional rendering for explore filters

14 files changed, 52 insertions, 63 deletions

diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb
index dfb741269eb..df2f4468199 100644
--- a/app/helpers/sorting_helper.rb
+++ b/app/helpers/sorting_helper.rb
@@ -30,8 +30,10 @@ module SortingHelper
   end
 
   def projects_sort_options_hash
-    is_admin = current_controller?('admin/projects')
+    Feature.enabled?(:project_list_filter_bar) && !current_controller?('admin/projects') ? projects_sort_common_options_hash : projects_sort_admin_options_hash
+  end
 
+  def projects_sort_admin_options_hash
     options = {
       sort_value_latest_activity  => sort_title_latest_activity,
       sort_value_name             => sort_title_name,
@@ -41,18 +43,20 @@ module SortingHelper
       sort_value_most_stars       => sort_title_most_stars
     }
 
-    if is_admin
+    if current_controller?('admin/projects')
       options[sort_value_largest_repo] = sort_title_largest_repo
     end
 
-    search_bar_options = {
+    options
+  end
+
+  def projects_sort_common_options_hash
+    {
       sort_value_latest_activity  => sort_title_latest_activity,
       sort_value_recently_created => sort_title_created_date,
       sort_value_name             => sort_title_name,
       sort_value_most_stars       => sort_title_stars
     }
-
-    Feature.enabled?(:project_list_filter_bar) && !is_admin ? search_bar_options : options
   end
 
   def projects_sort_option_titles
@@ -222,7 +226,7 @@ module SortingHelper
       link_class += ' disabled'
     end
 
-    link_to(reverse_url, type: 'button', class: link_class, title: 'Sort direction') do
+    link_to(reverse_url, type: 'button', class: link_class, title: s_('SortOptions|Sort direction')) do
       sprite_icon("sort-#{issuable_sort_icon_suffix(sort_value)}", size: 16)
     end
   end
@@ -238,7 +242,7 @@ module SortingHelper
       link_class += ' disabled'
     end
 
-    link_to(reverse_url, type: 'button', class: link_class, title: 'Sort direction') do
+    link_to(reverse_url, type: 'button', class: link_class, title: s_('SortOptions|Sort direction')) do
       sprite_icon("sort-#{issuable_sort_icon_suffix(sort_value)}", size: 16)
     end
   end
@@ -525,7 +529,6 @@ module SortingHelper
     'stars_desc'
   end
 
-  # TODO: currently not implemented AFAIK
   def sort_value_most_stars_asc
     'stars_asc'
   end
diff --git a/app/views/admin/projects/index.html.haml b/app/views/admin/projects/index.html.haml
index 46bb57c78a8..b88b760536d 100644
--- a/app/views/admin/projects/index.html.haml
+++ b/app/views/admin/projects/index.html.haml
@@ -7,7 +7,7 @@
   .top-area.scrolling-tabs-container.inner-page-scroll-tabs
     .prepend-top-default
       .search-holder
-        = render 'shared/projects/search_form', autofocus: true, icon: true
+        = render 'shared/projects/search_form', autofocus: true, icon: true, admin_view: true
         .dropdown
           - toggle_text = 'Namespace'
           - if params[:namespace_id].present?
diff --git a/app/views/dashboard/_projects_head.html.haml b/app/views/dashboard/_projects_head.html.haml
index e62ec3afd0b..128a766374e 100644
--- a/app/views/dashboard/_projects_head.html.haml
+++ b/app/views/dashboard/_projects_head.html.haml
@@ -1,6 +1,4 @@
-- is_explore = local_assigns.fetch(:is_explore, false)
-- is_explore_trending = local_assigns.fetch(:is_explore_trending, false)
-- without_tabs = local_assigns.fetch(:without_tabs, false)
+- project_tab_filter = local_assigns.fetch(:project_tab_filter, "")
 - feature_project_list_filter_bar = Feature.enabled?(:project_list_filter_bar)
 = content_for :flash_message do
   = render 'shared/project_limit'
@@ -33,4 +31,4 @@
       = render 'shared/projects/dropdown'
 - if feature_project_list_filter_bar
   .project-filters
-    = render 'shared/projects/search_bar', is_explore: is_explore, is_explore_trending: is_explore_trending, without_tabs: without_tabs
+    = render 'shared/projects/search_bar', project_tab_filter: project_tab_filter
diff --git a/app/views/dashboard/projects/_nav.html.haml b/app/views/dashboard/projects/_nav.html.haml
index b01e6c4293c..2da24e37c86 100644
--- a/app/views/dashboard/projects/_nav.html.haml
+++ b/app/views/dashboard/projects/_nav.html.haml
@@ -1,8 +1,7 @@
-
-- is_explore = local_assigns.fetch(:is_explore, false)
 - inactive_class = 'btn p-2'
 - active_class = 'btn p-2 active'
-- is_explore_trending = local_assigns.fetch(:is_explore_trending, false)
+- project_tab_filter = local_assigns.fetch(:project_tab_filter, "")
+- is_explore_trending = project_tab_filter == :explore_trending
 .nav-block{ class: Feature.enabled?(:project_list_filter_bar) ? "w-100" : "" }
   - if !Feature.enabled?(:project_list_filter_bar)
     %ul.nav-links.mobile-separator.nav.nav-tabs
@@ -12,7 +11,7 @@
         = link_to s_('DashboardProjects|Personal'), filter_projects_path(personal: true)
   - else
     .btn-group.button-filter-group.d-flex.m-0.p-0
-      - if is_explore
+      - if project_tab_filter == :explore || is_explore_trending
         = link_to s_('DashboardProjects|Trending'), trending_explore_projects_path, class: is_explore_trending ? active_class : inactive_class
         = link_to s_('DashboardProjects|All'), explore_projects_path, class: is_explore_trending ? inactive_class : active_class
       - else
diff --git a/app/views/dashboard/projects/starred.html.haml b/app/views/dashboard/projects/starred.html.haml
index a9075e462db..0fcc6894b68 100644
--- a/app/views/dashboard/projects/starred.html.haml
+++ b/app/views/dashboard/projects/starred.html.haml
@@ -3,13 +3,12 @@
 - breadcrumb_title _("Projects")
 - page_title _("Starred Projects")
 - header_title _("Projects"), dashboard_projects_path
-- without_tabs = true
 
 = render_dashboard_gold_trial(current_user)
 
 %div{ class: container_class }
   = render "projects/last_push"
-  = render 'dashboard/projects_head', without_tabs: without_tabs
+  = render 'dashboard/projects_head', project_tab_filter: :starred
 
   - if params[:filter_projects] || any_projects?(@projects)
     = render 'projects'
diff --git a/app/views/explore/projects/_filter.html.haml b/app/views/explore/projects/_filter.html.haml
index bd21ba6cc71..4606f3ec674 100644
--- a/app/views/explore/projects/_filter.html.haml
+++ b/app/views/explore/projects/_filter.html.haml
@@ -1,4 +1,4 @@
-- has_label = local_assigns[:has_label] ? local_assigns[:has_label] : false
+- has_label = local_assigns.fetch(:has_label, false)
 - if current_user
   .dropdown.js-project-filter-dropdown-wrap
     %button.dropdown-menu-toggle{ href: '#', "data-toggle" => "dropdown", 'data-display' => 'static' }
diff --git a/app/views/explore/projects/_nav.html.haml b/app/views/explore/projects/_nav.html.haml
index e4e8d4fc008..9ec2122d5a3 100644
--- a/app/views/explore/projects/_nav.html.haml
+++ b/app/views/explore/projects/_nav.html.haml
@@ -11,10 +11,8 @@
         = _('All')
 
   .nav-controls
-    -# TODO: filters are currently only shown when logged out, I assume we want to show them all the time?
     - unless current_user
       = render 'shared/projects/search_form'
       = render 'shared/projects/dropdown'
-    - unless Feature.enabled?(:project_list_filter_bar)
-      = render 'filter'
+    = render 'filter'
 
diff --git a/app/views/explore/projects/index.html.haml b/app/views/explore/projects/index.html.haml
index fed226a184c..341ad681c7c 100644
--- a/app/views/explore/projects/index.html.haml
+++ b/app/views/explore/projects/index.html.haml
@@ -5,10 +5,9 @@
 = render_dashboard_gold_trial(current_user)
 
 - if current_user
-  = render 'dashboard/projects_head', is_explore: true
+  = render 'dashboard/projects_head', project_tab_filter: :explore
 - else
   = render 'explore/head'
 
-- unless Feature.enabled?(:project_list_filter_bar)
-  = render 'explore/projects/nav'
+= render 'explore/projects/nav' unless Feature.enabled?(:project_list_filter_bar) && current_user
 = render 'projects', projects: @projects
diff --git a/app/views/explore/projects/starred.html.haml b/app/views/explore/projects/starred.html.haml
index fed226a184c..ec92852ddde 100644
--- a/app/views/explore/projects/starred.html.haml
+++ b/app/views/explore/projects/starred.html.haml
@@ -5,10 +5,9 @@
 = render_dashboard_gold_trial(current_user)
 
 - if current_user
-  = render 'dashboard/projects_head', is_explore: true
+  = render 'dashboard/projects_head', project_tab_filter: :starred
 - else
   = render 'explore/head'
 
-- unless Feature.enabled?(:project_list_filter_bar)
-  = render 'explore/projects/nav'
+= render 'explore/projects/nav' unless Feature.enabled?(:project_list_filter_bar) && current_user
 = render 'projects', projects: @projects
diff --git a/app/views/explore/projects/trending.html.haml b/app/views/explore/projects/trending.html.haml
index e4e612175f5..ed508fa2506 100644
--- a/app/views/explore/projects/trending.html.haml
+++ b/app/views/explore/projects/trending.html.haml
@@ -5,10 +5,9 @@
 = render_dashboard_gold_trial(current_user)
 
 - if current_user
-  = render 'dashboard/projects_head', is_explore: true, is_explore_trending: true
+  = render 'dashboard/projects_head', project_tab_filter: :explore_trending
 - else
   = render 'explore/head'
 
-- unless Feature.enabled?(:project_list_filter_bar)
-  = render 'explore/projects/nav'
+= render 'explore/projects/nav' unless Feature.enabled?(:project_list_filter_bar) && current_user
 = render 'projects', projects: @projects
diff --git a/app/views/shared/groups/_dropdown.html.haml b/app/views/shared/groups/_dropdown.html.haml
index 1ae6d1f5ee3..f4915440cb2 100644
--- a/app/views/shared/groups/_dropdown.html.haml
+++ b/app/views/shared/groups/_dropdown.html.haml
@@ -24,10 +24,10 @@
       %li.divider
       %li.js-filter-archived-projects
         = link_to filter_groups_path(archived: nil), class: ("is-active" unless params[:archived].present?) do
-          Hide archived projects
+          = _("Hide archived projects")
       %li.js-filter-archived-projects
         = link_to filter_groups_path(archived: true), class: ("is-active" if Gitlab::Utils.to_boolean(params[:archived])) do
-          Show archived projects
+          = _("Show archived projects")
       %li.js-filter-archived-projects
         = link_to filter_groups_path(archived: 'only'), class: ("is-active" if params[:archived] == 'only') do
-          Show archived projects only
+          = _("Show archived projects only")
diff --git a/app/views/shared/projects/_dropdown.html.haml b/app/views/shared/projects/_dropdown.html.haml
index 98b258d9275..88ac03bf9e3 100644
--- a/app/views/shared/projects/_dropdown.html.haml
+++ b/app/views/shared/projects/_dropdown.html.haml
@@ -1,10 +1,9 @@
 - @sort ||= sort_value_latest_activity
 .dropdown.js-project-filter-dropdown-wrap
-  - toggle_text = projects_sort_options_hash[@sort]
-  = dropdown_toggle(toggle_text, { toggle: 'dropdown', display: 'static' }, { id: 'sort-projects-dropdown' })
+  = dropdown_toggle(projects_sort_options_hash[@sort], { toggle: 'dropdown', display: 'static' }, { id: 'sort-projects-dropdown' })
   %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-selectable
     %li.dropdown-header
-      Sort by
+      = _("Sort by")
     - projects_sort_options_hash.each do |value, title|
       %li
         = link_to filter_projects_path(sort: value), class: ("is-active" if @sort == value) do
@@ -13,29 +12,29 @@
     %li.divider
     %li
       = link_to filter_projects_path(archived: nil), class: ("is-active" unless params[:archived].present?) do
-        Hide archived projects
+        = _("Hide archived projects")
     %li
       = link_to filter_projects_path(archived: true), class: ("is-active" if Gitlab::Utils.to_boolean(params[:archived])) do
-        Show archived projects
+        = _("Show archived projects")
     %li
       = link_to filter_projects_path(archived: 'only'), class: ("is-active" if params[:archived] == 'only') do
-        Show archived projects only
+        = _("Show archived projects only")
     - if current_user
       %li.divider
       %li
         = link_to filter_projects_path(personal: nil), class: ("is-active" unless params[:personal].present?) do
-          Owned by anyone
+          = _("Owned by anyone")
       %li
         = link_to filter_projects_path(personal: true), class: ("is-active" if params[:personal].present?) do
-          Owned by me
+          = _("Owned by me")
       - if @group && @group.shared_projects.present?
         %li.divider
         %li
           = link_to filter_projects_path(shared: nil), class: ("is-active" unless params[:shared].present?) do
-            All projects
+            = _("All projects")
         %li
           = link_to filter_projects_path(shared: 0), class: ("is-active" if params[:shared] == '0') do
-            Hide shared projects
+            = _("Hide shared projects")
         %li
           = link_to filter_projects_path(shared: 1), class: ("is-active" if params[:shared] == '1') do
-            Hide group projects
+            = _("Hide group projects")
diff --git a/app/views/shared/projects/_search_bar.html.haml b/app/views/shared/projects/_search_bar.html.haml
index a70e60d7948..e44d8692eea 100644
--- a/app/views/shared/projects/_search_bar.html.haml
+++ b/app/views/shared/projects/_search_bar.html.haml
@@ -1,20 +1,16 @@
 - @sort ||= sort_value_latest_activity
-- is_explore = local_assigns.fetch(:is_explore, false)
-- is_explore_trending = local_assigns.fetch(:is_explore_trending, false)
-- without_tabs = local_assigns.fetch(:without_tabs, false)
+- project_tab_filter = local_assigns.fetch(:project_tab_filter, "")
 .filtered-search-block.row-content-block.bt-0
   .filtered-search-wrapper.d-flex.flex-nowrap.flex-column.flex-sm-wrap.flex-sm-row.flex-xl-nowrap
-    - unless without_tabs
+    - unless project_tab_filter == :starred
       .filtered-search-nav.d-flex.mb-2.mb-lg-0
-        = render 'dashboard/projects/nav', is_explore: is_explore, is_explore_trending: is_explore_trending
-    .filtered-search.d-flex.w-100.mb-2.mb-lg-0{ class: without_tabs ? "extended-filtered-search-box ml-0 mb-2 mb-lg-0" : "ml-0 ml-sm-3" }
+        = render 'dashboard/projects/nav', project_tab_filter: project_tab_filter
+    .filtered-search.d-flex.w-100.mb-2.mb-lg-0{ class: project_tab_filter == :starred ? "extended-filtered-search-box ml-0 mb-2 mb-lg-0" : "ml-0 ml-sm-3" }
       .btn-group.w-100{ role: "group" }
         .btn-group.w-100{ role: "group" }
           .filtered-search-box.m-0
             .filtered-search-box-input-container.pl-2
               = render 'shared/projects/search_form', admin_view: false, search_form_placeholder: _("Search projects...")
-        -# TODO: since we are no longer triggering search when we type
-        -#       we might be able to remove app/assets/javascripts/projects_list.js
         %button.btn.btn-secondary{ type: 'submit', form: 'project-filter-form' }
           = sprite_icon('search', size: 16, css_class: 'search-icon ')
     .filtered-search-dropdown.d-flex.flex-row.align-items-center.mb-2.m-sm-0#filtered-search-visibility-dropdown
diff --git a/app/views/shared/projects/_sort_dropdown.html.haml b/app/views/shared/projects/_sort_dropdown.html.haml
index c806d0851db..65f175efcf7 100644
--- a/app/views/shared/projects/_sort_dropdown.html.haml
+++ b/app/views/shared/projects/_sort_dropdown.html.haml
@@ -1,38 +1,38 @@
 - @sort ||= sort_value_latest_activity
+- toggle_text = projects_sort_option_titles[@sort]
 .btn-group.w-100{ role: "group" }
   .btn-group.w-100.dropdown.js-project-filter-dropdown-wrap{ role: "group" }
-    - toggle_text = projects_sort_option_titles[@sort]
     %button.dropdown-menu-toggle{ id: 'sort-projects-dropdown', type: 'button', data: { toggle: 'dropdown', display: 'static' }, class: 'btn btn-default' }
       = toggle_text
       = icon('chevron-down')
     %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-selectable
       %li.dropdown-header
-        Sort by
+        = _("Sort by")
       - projects_sort_options_hash.each do |value, title|
         %li
-          = link_to filter_projects_path(sort: value), class: ("is-active" if projects_sort_option_titles[@sort] == title) do
+          = link_to filter_projects_path(sort: value), class: ("is-active" if toggle_text == title) do
             = title
 
       %li.divider
       %li
         = link_to filter_projects_path(archived: nil), class: ("is-active" unless params[:archived].present?) do
-          Hide archived projects
+          = _("Hide archived projects")
       %li
         = link_to filter_projects_path(archived: true), class: ("is-active" if Gitlab::Utils.to_boolean(params[:archived])) do
-          Show archived projects
+          = _("Show archived projects")
       %li
         = link_to filter_projects_path(archived: 'only'), class: ("is-active" if params[:archived] == 'only') do
-          Show archived projects only
+          = _("Show archived projects only")
       - if current_user && @group && @group.shared_projects.present?
         %li.divider
         %li
           = link_to filter_projects_path(shared: nil), class: ("is-active" unless params[:shared].present?) do
-            All projects
+            = _("All projects")
         %li
           = link_to filter_projects_path(shared: 0), class: ("is-active" if params[:shared] == '0') do
-            Hide shared projects
+            = _("Hide shared projects")
         %li
           = link_to filter_projects_path(shared: 1), class: ("is-active" if params[:shared] == '1') do
-            Hide group projects
+            = _("Hide group projects")
 
   = project_sort_direction_button(@sort)