Merge branch 'dz-redesign-group-settings-page' into 'master'

Redesign group settings page into expandable sections

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

app/assets/javascripts/pages/groups/edit/index.js

 import groupAvatar from '~/group_avatar';
 import TransferDropdown from '~/groups/transfer_dropdown';
 import initConfirmDangerModal from '~/confirm_danger_modal';
+import initSettingsPanels from '~/settings_panels';
 
 document.addEventListener('DOMContentLoaded', () => {
   groupAvatar();
   new TransferDropdown(); // eslint-disable-line no-new
   initConfirmDangerModal();
 });
+
+document.addEventListener('DOMContentLoaded', () => {
+  // Initialize expandable settings panels
+  initSettingsPanels();
+});

app/views/groups/edit.html.haml

 - breadcrumb_title "General Settings"
 - @content_class = "limit-container-width" unless fluid_layout
+- expanded = Rails.env.test?
 
-.card.prepend-top-default
-  .card-header
-    Group settings
-  .card-body
-    = form_for @group, html: { multipart: true, class: "gl-show-field-errors" }, authenticity_token: true do |f|
-      = form_errors(@group)
-      = render 'shared/group_form', f: f
 
-      .form-group.row
-        .offset-sm-2.col-sm-10
-          .avatar-container.s160
-            = group_icon(@group, alt: '', class: 'avatar group-avatar s160')
-          %p.light
-            - if @group.avatar?
-              You can change the group avatar here
-            - else
-              You can upload a group avatar here
-          = render 'shared/choose_group_avatar_button', f: f
-          - if @group.avatar?
-            %hr
-            = link_to _('Remove avatar'), group_avatar_path(@group.to_param), data: { confirm: _("Avatar will be removed. Are you sure?")}, method: :delete, class: "btn btn-danger btn-inverted"
-
-      = render 'shared/visibility_level', f: f, visibility_level: @group.visibility_level, can_change_visibility_level: can_change_group_visibility_level?(@group), form_model: @group
-
-      .form-group.row
-        .offset-sm-2.col-sm-10
-          = render 'shared/allow_request_access', form: f
-
-      .form-group.row
-        %label.col-form-label.col-sm-2
-          = s_("GroupSettings|Share with group lock")
-        .col-sm-10
-          .form-check
-            = f.label :share_with_group_lock do
-              = f.check_box :share_with_group_lock, disabled: !can_change_share_with_group_lock?(@group)
-              %strong
-                - group_link = link_to @group.name, group_path(@group)
-                = s_("GroupSettings|Prevent sharing a project within %{group} with other groups").html_safe % { group: group_link }
-              %br
-              %span.descr= share_with_group_lock_help_text(@group)
-
-      = render 'group_admin_settings', f: f
-
-      .form-actions
-        = f.submit 'Save group', class: "btn btn-save"
-
-.card.bg-danger
-  .card-header Remove group
-  .card-body
-    = form_tag(@group, method: :delete) do
+%section.settings.gs-general.no-animate#js-general-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('General')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? _('Collapse') : _('Expand')
     %p
-        Removing group will cause all child projects and resources to be removed.
-        %br
-        %strong Removed group can not be restored!
-
-      .form-actions
-        = button_to 'Remove group', '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_group_message(@group) }
-
-- if supports_nested_groups?
-  .card.bg-warning
-    .card-header Transfer group
-    .card-body
-      = form_for @group, url: transfer_group_path(@group), method: :put do |f|
-        .form-group
-          = dropdown_tag('Select parent group', options: { toggle_class: 'js-groups-dropdown', title: 'Parent Group', filter: true, dropdown_class: 'dropdown-open-top dropdown-group-transfer', placeholder: "Search groups", data: { data: parent_group_options(@group) } })
-          = hidden_field_tag 'new_parent_group_id'
-
-        %ul
-          %li Be careful. Changing a group's parent can have unintended #{link_to 'side effects', 'https://docs.gitlab.com/ce/user/project/index.html#redirects-when-changing-repository-paths', target: 'blank'}.
-          %li You can only transfer the group to a group you manage.
-          %li You will need to update your local repositories to point to the new location.
-          %li If the parent group's visibility is lower than the group current visibility, visibility levels for subgroups and projects will be changed to match the new parent group's visibility.
-        = f.submit 'Transfer group', class: "btn btn-warning"
+      = _('Update your group name, description, avatar, and other general settings.')
+  .settings-content
+    = render 'groups/settings/general'
+
+%section.settings.gs-permissions.no-animate#js-permissions-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('Permissions')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? _('Collapse') : _('Expand')
+    %p
+      = _('Enable or disable certain group features and choose access levels.')
+  .settings-content
+    = render 'groups/settings/permissions'
+
+%section.settings.gs-advanced.no-animate#js-advanced-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('Advanced')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? _('Collapse') : _('Expand')
+    %p
+      = _('Perform advanced options such as changing path, transferring, or removing the group.')
+  .settings-content
+    = render 'groups/settings/advanced'
 
 = render 'shared/confirm_modal', phrase: @group.path

app/views/groups/settings/_advanced.html.haml

+.sub-section
+  %h4.warning-title Change group path
+  = form_for @group, html: { multipart: true, class: 'gl-show-field-errors' }, authenticity_token: true do |f|
+    = form_errors(@group)
+    .form-group
+      %p
+        Changing group path can have unintended side effects.
+        = succeed '.' do
+          = link_to 'Learn more', help_page_path('user/group/index', anchor: 'changing-a-groups-path'), target: '_blank'
+
+      .input-group.gl-field-error-anchor
+        .group-root-path.input-group-prepend.has-tooltip{ title: group_path(@group), :'data-placement' => 'bottom' }
+          .input-group-text
+            %span>= root_url
+            - if parent
+              %strong= parent.full_path + '/'
+        = f.hidden_field :parent_id
+        = f.text_field :path, placeholder: 'open-source', class: 'form-control',
+          autofocus: local_assigns[:autofocus] || false, required: true,
+          pattern: Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX_JS,
+          title: 'Please choose a group path with no special characters.',
+          "data-bind-in" => "#{'create_chat_team' if Gitlab.config.mattermost.enabled}"
+
+    = f.submit 'Change group path', class: 'btn btn-warning'
+
+.sub-section
+  %h4.danger-title Remove group
+  = form_tag(@group, method: :delete) do
+    %p
+      Removing group will cause all child projects and resources to be removed.
+      %br
+      %strong Removed group can not be restored!
+
+    = button_to 'Remove group', '#', class: 'btn btn-remove js-confirm-danger', data: { 'confirm-danger-message' => remove_group_message(@group) }
+
+- if supports_nested_groups?
+  .sub-section
+    %h4.warning-title Transfer group
+    = form_for @group, url: transfer_group_path(@group), method: :put do |f|
+      .form-group
+        = dropdown_tag('Select parent group', options: { toggle_class: 'js-groups-dropdown', title: 'Parent Group', filter: true, dropdown_class: 'dropdown-open-top dropdown-group-transfer', placeholder: 'Search groups', data: { data: parent_group_options(@group) } })
+        = hidden_field_tag 'new_parent_group_id'
+
+      %ul
+        %li Be careful. Changing a group's parent can have unintended #{link_to 'side effects', 'https://docs.gitlab.com/ce/user/project/index.html#redirects-when-changing-repository-paths', target: 'blank'}.
+        %li You can only transfer the group to a group you manage.
+        %li You will need to update your local repositories to point to the new location.
+        %li If the parent group's visibility is lower than the group current visibility, visibility levels for subgroups and projects will be changed to match the new parent group's visibility.
+      = f.submit 'Transfer group', class: 'btn btn-warning'

app/views/groups/settings/_general.html.haml

+= form_for @group, html: { multipart: true, class: 'gl-show-field-errors' }, authenticity_token: true do |f|
+  = form_errors(@group)
+
+  %fieldset
+    .row
+      .form-group.col-md-9
+        = f.label :name, class: 'label-light' do
+          Group name
+        = f.text_field :name, class: 'form-control'
+
+      .form-group.col-md-3
+        = f.label :id, class: 'label-light' do
+          Group ID
+        = f.text_field :id, class: 'form-control', readonly: true
+
+    .form-group
+      = f.label :description, class: 'label-light' do
+        Group description
+        %span.light (optional)
+      = f.text_area :description, class: 'form-control', rows: 3, maxlength: 250
+
+    = render_if_exists 'shared/repository_size_limit_setting', form: f, type: :group
+
+    .form-group.row
+      .col-sm-12
+        .avatar-container.s160
+          = group_icon(@group, alt: '', class: 'avatar group-avatar s160')
+        %p.light
+          - if @group.avatar?
+            You can change the group avatar here
+          - else
+            You can upload a group avatar here
+        = render 'shared/choose_group_avatar_button', f: f
+        - if @group.avatar?
+          %hr
+          = link_to _('Remove avatar'), group_avatar_path(@group.to_param), data: { confirm: _('Avatar will be removed. Are you sure?')}, method: :delete, class: 'btn btn-danger btn-inverted'
+
+  = f.submit 'Save group', class: 'btn btn-success'

app/views/groups/settings/_permissions.html.haml

+= form_for @group, html: { multipart: true, class: 'gl-show-field-errors' }, authenticity_token: true do |f|
+  = form_errors(@group)
+
+  %fieldset
+    = render 'shared/visibility_level', f: f, visibility_level: @group.visibility_level, can_change_visibility_level: can_change_group_visibility_level?(@group), form_model: @group
+
+    .form-group.row
+      .offset-sm-2.col-sm-10
+        = render 'shared/allow_request_access', form: f
+
+    .form-group.row
+      %label.col-form-label.col-sm-2
+        = s_('GroupSettings|Share with group lock')
+      .col-sm-10
+        .form-check
+          = f.label :share_with_group_lock do
+            = f.check_box :share_with_group_lock, disabled: !can_change_share_with_group_lock?(@group)
+            %strong
+              - group_link = link_to @group.name, group_path(@group)
+              = s_('GroupSettings|Prevent sharing a project within %{group} with other groups').html_safe % { group: group_link }
+            %br
+            %span.descr= share_with_group_lock_help_text(@group)
+
+    = render 'groups/group_admin_settings', f: f
+
+    = render_if_exists 'groups/member_lock_setting', f: f, group: @group
+
+  = f.submit 'Save group', class: 'btn btn-success'

changelogs/unreleased/dz-redesign-group-settings-page.yml

+---
+title: Redesign group settings page into expandable sections
+merge_request: 19184
+author:
+type: changed

spec/features/groups/group_settings_spec.rb

@@ -83,7 +83,7 @@ feature 'Edit group settings' do
 
       attach_file(:group_avatar, Rails.root.join('spec', 'fixtures', 'banana_sample.gif'))
 
-      expect { click_button 'Save group' }.to change { group.reload.avatar? }.to(true)
+      expect { save_group }.to change { group.reload.avatar? }.to(true)
     end
 
     it 'uploads new group avatar' do
@@ -97,10 +97,19 @@ feature 'Edit group settings' do
       expect(page).not_to have_link('Remove avatar')
     end
   end
-end
 
-def update_path(new_group_path)
+  def update_path(new_group_path)
     visit edit_group_path(group)
+
+    page.within('.gs-advanced') do
       fill_in 'group_path', with: new_group_path
+      click_button 'Change group path'
+    end
+  end
+
+  def save_group
+    page.within('.gs-general') do
       click_button 'Save group'
+    end
+  end
 end

spec/features/groups/share_lock_spec.rb

@@ -15,9 +15,8 @@ feature 'Group share with group lock' do
     context 'when enabling the parent group share with group lock' do
       scenario 'the subgroup share with group lock becomes enabled' do
         visit edit_group_path(root_group)
-        check 'group_share_with_group_lock'
 
-        click_on 'Save group'
+        enable_group_lock
 
         expect(subgroup.reload.share_with_group_lock?).to be_truthy
       end
@@ -26,16 +25,15 @@ feature 'Group share with group lock' do
     context 'when disabling the parent group share with group lock (which was already enabled)' do
       background do
         visit edit_group_path(root_group)
-        check 'group_share_with_group_lock'
-        click_on 'Save group'
+
+        enable_group_lock
       end
 
       context 'and the subgroup share with group lock is enabled' do
         scenario 'the subgroup share with group lock does not change' do
           visit edit_group_path(root_group)
-          uncheck 'group_share_with_group_lock'
 
-          click_on 'Save group'
+          disable_group_lock
 
           expect(subgroup.reload.share_with_group_lock?).to be_truthy
         end
@@ -44,19 +42,32 @@ feature 'Group share with group lock' do
       context 'but the subgroup share with group lock is disabled' do
         background do
           visit edit_group_path(subgroup)
-          uncheck 'group_share_with_group_lock'
-          click_on 'Save group'
+
+          disable_group_lock
         end
 
         scenario 'the subgroup share with group lock does not change' do
           visit edit_group_path(root_group)
-          uncheck 'group_share_with_group_lock'
 
-          click_on 'Save group'
+          disable_group_lock
 
           expect(subgroup.reload.share_with_group_lock?).to be_falsey
         end
       end
     end
   end
+
+  def enable_group_lock
+    page.within('.gs-permissions') do
+      check 'group_share_with_group_lock'
+      click_on 'Save group'
+    end
+  end
+
+  def disable_group_lock
+    page.within('.gs-permissions') do
+      uncheck 'group_share_with_group_lock'
+      click_on 'Save group'
+    end
+  end
 end

spec/features/groups_spec.rb

@@ -141,8 +141,10 @@ feature 'Group' do
     end
 
     it 'saves new settings' do
+      page.within('.gs-general') do
         fill_in 'group_name', with: new_name
         click_button 'Save group'
+      end
 
       expect(page).to have_content 'successfully updated'
       expect(find('#group_name').value).to eq(new_name)

spec/features/uploads/user_uploads_avatar_to_group_spec.rb

@@ -14,7 +14,9 @@ feature 'User uploads avatar to group' do
       visible: false
     )
 
+    page.within('.gs-general') do
       click_button 'Save group'
+    end
 
     visit group_path(group)