Created shared/visibility_select and merged with feature permissions ui

Fixed up nested padding and added dynamic text js

Added padding and tests

7 files changed, 154 insertions, 59 deletions

diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6
index 413117c2226..237a7dab5e9 100644
--- a/app/assets/javascripts/dispatcher.js.es6
+++ b/app/assets/javascripts/dispatcher.js.es6
@@ -262,6 +262,7 @@
             case 'edit':
               shortcut_handler = new ShortcutsNavigation();
               new ProjectNew();
+              new gl.VisibilitySelect();
               break;
             case 'new':
               new ProjectNew();
diff --git a/app/assets/javascripts/visibility_select.js.es6 b/app/assets/javascripts/visibility_select.js.es6
new file mode 100644
index 00000000000..a2f8a2395c1
--- /dev/null
+++ b/app/assets/javascripts/visibility_select.js.es6
@@ -0,0 +1,29 @@
+(() => {
+  const global = window.gl || (window.gl = {});
+
+  const VISIBILITY_DESCRIPTIONS = {
+    0: 'Project access must be granted explicitly to each user.',
+    10: 'Project access must be granted explicitly to each user.',
+    20: 'The project can be cloned without any authentication.',
+  };
+
+  class VisibilitySelect {
+    constructor() {
+      this.visibilitySelect = document.querySelector('.js-visibility-select');
+      this.helpBlock = this.visibilitySelect.querySelector('.help-block');
+      this.select = this.visibilitySelect.querySelector('select');
+      if (this.select) {
+        this.visibilityChanged();
+        this.select.addEventListener('change', this.visibilityChanged.bind(this));
+      } else {
+        this.helpBlock.textContent = this.visibilitySelect.querySelector('.js-locked').dataset.helpBlock;
+      }
+    }
+
+    visibilityChanged() {
+      this.helpBlock.innerText = VISIBILITY_DESCRIPTIONS[this.select.value];
+    }
+  }
+
+  global.VisibilitySelect = VisibilitySelect;
+})();
diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss
index 72b6685d940..dcb327a67c6 100644
--- a/app/assets/stylesheets/pages/projects.scss
+++ b/app/assets/stylesheets/pages/projects.scss
@@ -15,10 +15,22 @@
 .new_project,
 .edit-project {
 
+  .visibility-select-container {
+    padding-left: 26px;
+
+    @media(max-width: $screen-md-min) {
+      padding-left: 15px;
+    }
+  }
+
   fieldset {
 
     &.features {
 
+      .header {
+        padding-top: $gl-vert-padding;
+      }
+
       .label-light {
         margin-bottom: 0;
       }
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index 38e7fc4279c..16a26f7d066 100644
--- a/app/views/projects/edit.html.haml
+++ b/app/views/projects/edit.html.haml
@@ -21,76 +21,69 @@
             .form-group
               = f.label :default_branch, "Default Branch", class: 'label-light'
               = f.select(:default_branch, @project.repository.branch_names, {}, {class: 'select2 select-wide'})
-        .form-group.project-visibility-level-holder
-          = f.label :visibility_level, class: 'label-light' do
-            Visibility Level
-            = link_to "(?)", help_page_path("public_access/public_access")
-          - if can_change_visibility_level?(@project, current_user)
-            = render('shared/visibility_radios', model_method: :visibility_level, form: f, selected_level: @project.visibility_level, form_model: @project)
-          - else
-            .info
-              = visibility_level_icon(@project.visibility_level)
-              %strong
-                = visibility_level_label(@project.visibility_level)
-              .light= visibility_level_description(@project.visibility_level, @project)
-
-        .form-group
-          = render 'shared/allow_request_access', form: f
-
         .form-group
           = f.label :tag_list, "Tags", class: 'label-light'
           = f.text_field :tag_list, value: @project.tag_list.to_s, maxlength: 2000, class: "form-control"
           %p.help-block Separate tags with commas.
         %hr
-        %fieldset.features.append-bottom-0
+        %fieldset.append-bottom-0
           %h5.prepend-top-0
-            Feature Visibility
-
-          = f.fields_for :project_feature do |feature_fields|
-            .form_group.prepend-top-20
-              .row
-                .col-md-9
-                  = feature_fields.label :repository_access_level, "Repository", class: 'label-light'
-                  %span.help-block Push files to be stored in this project
-                .col-md-3.js-repo-access-level
-                  = project_feature_access_select(:repository_access_level)
-
-                .col-sm-12
-                  .row
-                    .col-md-9.project-feature-nested
-                      = feature_fields.label :merge_requests_access_level, "Merge requests", class: 'label-light'
-                      %span.help-block Submit changes to be merged upstream
-                    .col-md-3
-                      = project_feature_access_select(:merge_requests_access_level)
+            Sharing & Permissions
+          .form_group.prepend-top-20
+            .row.js-visibility-select
+              .col-md-9
+                %label.label-light
+                  = label_tag :project_visibility, 'Project Visibility', class: 'label-light'
+                  = link_to "(?)", help_page_path("public_access/public_access")
+                %span.help-block
+              .col-md-3.visibility-select-container
+                = render('shared/visibility_select', model_method: :visibility_level, form: f, selected_level: @project.visibility_level)
+            = f.fields_for :project_feature do |feature_fields|
+              %fieldset.features
+                .row.project-feature-nested
+                  .col-md-9.header
+                    = feature_fields.label :repository_access_level, "Repository", class: 'label-light'
+                    %span.help-block Push files to be stored in this project
+                  .col-md-3.js-repo-access-level
+                    = project_feature_access_select(:repository_access_level)
 
-                  .row
-                    .col-md-9.project-feature-nested
-                      = feature_fields.label :builds_access_level, "Builds", class: 'label-light'
-                      %span.help-block Submit, test and deploy your changes before merge
-                    .col-md-3
-                      = project_feature_access_select(:builds_access_level)
+                  .col-sm-12
+                    .row.project-feature-nested
+                      .col-md-9.header
+                        = feature_fields.label :merge_requests_access_level, "Merge requests", class: 'label-light'
+                        %span.help-block Submit changes to be merged upstream
+                      .col-md-3
+                        = project_feature_access_select(:merge_requests_access_level)
 
-              .row
-                .col-md-9
-                  = feature_fields.label :snippets_access_level, "Snippets", class: 'label-light'
-                  %span.help-block Share code pastes with others out of Git repository
-                .col-md-3
-                  = project_feature_access_select(:snippets_access_level)
+                    .row.project-feature-nested
+                      .col-md-9.header
+                        = feature_fields.label :builds_access_level, "Builds", class: 'label-light'
+                        %span.help-block Submit, test and deploy your changes before merge
+                      .col-md-3.double-nested
+                        = project_feature_access_select(:builds_access_level)
 
-              .row
-                .col-md-9
-                  = feature_fields.label :issues_access_level, "Issues", class: 'label-light'
-                  %span.help-block Lightweight issue tracking system for this project
-                .col-md-3
-                  = project_feature_access_select(:issues_access_level)
+                    .row
+                      .col-md-9.header
+                        = feature_fields.label :snippets_access_level, "Snippets", class: 'label-light'
+                        %span.help-block Share code pastes with others out of Git repository
+                      .col-md-3
+                        = project_feature_access_select(:snippets_access_level)
 
-              .row
-                .col-md-9
-                  = feature_fields.label :wiki_access_level, "Wiki", class: 'label-light'
-                  %span.help-block Pages for project documentation
-                .col-md-3
-                  = project_feature_access_select(:wiki_access_level)
+                    .row
+                      .col-md-9.header
+                        = feature_fields.label :issues_access_level, "Issues", class: 'label-light'
+                        %span.help-block Lightweight issue tracking system for this project
+                      .col-md-3
+                        = project_feature_access_select(:issues_access_level)
 
+                    .row
+                      .col-md-9.header
+                        = feature_fields.label :wiki_access_level, "Wiki", class: 'label-light'
+                        %span.help-block Pages for project documentation
+                      .col-md-3
+                        = project_feature_access_select(:wiki_access_level)
+          .form-group
+            = render 'shared/allow_request_access', form: f
           - if Gitlab.config.lfs.enabled && current_user.admin?
             .row
               .col-md-9
diff --git a/app/views/shared/_visibility_select.html.haml b/app/views/shared/_visibility_select.html.haml
new file mode 100644
index 00000000000..42210f565f6
--- /dev/null
+++ b/app/views/shared/_visibility_select.html.haml
@@ -0,0 +1,11 @@
+- if can_change_visibility_level?(@project, current_user)
+  - levels_options_hash = {}
+  - Gitlab::VisibilityLevel.values.each do |level|
+    - levels_options_hash[visibility_level_label(level)] = level
+  - options = options_for_select(levels_options_hash, selected_level)
+  = form.select(model_method, options, {}, class: 'form-control visibility-select')
+- else
+  .info.js-locked{ data: { help_block: visibility_level_description(@project.visibility_level, @project)}}
+    = visibility_level_icon(@project.visibility_level)
+    %strong
+      = visibility_level_label(@project.visibility_level)
diff --git a/changelogs/unreleased/24032-changed-visibility-level-to-public-but-project-is-not-public.yml b/changelogs/unreleased/24032-changed-visibility-level-to-public-but-project-is-not-public.yml
new file mode 100644
index 00000000000..875106d7dd5
--- /dev/null
+++ b/changelogs/unreleased/24032-changed-visibility-level-to-public-but-project-is-not-public.yml
@@ -0,0 +1,4 @@
+---
+title: Updated project visibility settings UX
+merge_request: 7645
+author: 
diff --git a/spec/features/projects/settings/visibility_settings_spec.rb b/spec/features/projects/settings/visibility_settings_spec.rb
new file mode 100644
index 00000000000..86cc3fb886e
--- /dev/null
+++ b/spec/features/projects/settings/visibility_settings_spec.rb
@@ -0,0 +1,45 @@
+require 'spec_helper'
+require 'byebug'
+
+feature 'Visibility settings', feature: true, js: true do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, namespace: user.namespace, visibility_level: 20) }
+
+  context 'as owner' do
+    before do
+      login_as(user)
+      visit edit_namespace_project_path(project.namespace, project)
+    end
+
+    scenario 'project visibility select is available' do
+      visibility_select_container = find('.js-visibility-select')
+      expect(visibility_select_container.find('.visibility-select').value).to eq project.visibility_level.to_s
+      expect(visibility_select_container).to have_content 'The project can be cloned without any authentication.'
+    end
+
+    scenario 'project visibility description updates on change' do
+      visibility_select_container = find('.js-visibility-select')
+      visibility_select = visibility_select_container.find('.visibility-select')
+      visibility_select.select('Private')
+      expect(visibility_select.value).to eq '0'
+      expect(visibility_select_container).to have_content 'Project access must be granted explicitly to each user.'
+    end
+  end
+
+  context 'as master' do
+    let(:master_user) { create(:user) }
+
+    before do
+      project.team << [master_user, :master]
+      login_as(master_user)
+      visit edit_namespace_project_path(project.namespace, project)
+    end
+
+    scenario 'project visibility is locked' do
+      visibility_select_container = find('.js-visibility-select')
+      expect(visibility_select_container).not_to have_select '.visibility-select'
+      expect(visibility_select_container).to have_content 'Public'
+      expect(visibility_select_container).to have_content 'The project can be cloned without any authentication.'
+    end
+  end
+end