Merge branch...

Merge branch 'ee-49417-improve-settings-pages-design-by-prioritizing-content-group-settings' into 'master'

EE port gitlab-ce!21115

See merge request gitlab-org/gitlab-ee!7720

app/assets/javascripts/dirty_submit/dirty_submit_collection.js

+import DirtySubmitForm from './dirty_submit_form';
+
+class DirtySubmitCollection {
+  constructor(forms) {
+    this.forms = forms;
+
+    this.dirtySubmits = [];
+
+    this.forms.forEach(form => this.dirtySubmits.push(new DirtySubmitForm(form)));
+  }
+}
+
+export default DirtySubmitCollection;

app/assets/javascripts/dirty_submit/dirty_submit_factory.js

+import DirtySubmitCollection from './dirty_submit_collection';
+import DirtySubmitForm from './dirty_submit_form';
+
+export default function dirtySubmitFactory(formOrForms) {
+  const isCollection = formOrForms instanceof NodeList || formOrForms instanceof Array;
+  const DirtySubmitClass = isCollection ? DirtySubmitCollection : DirtySubmitForm;
+
+  return new DirtySubmitClass(formOrForms);
+}

app/assets/javascripts/dirty_submit/dirty_submit_form.js

+import _ from 'underscore';
+
+class DirtySubmitForm {
+  constructor(form) {
+    this.form = form;
+    this.dirtyInputs = [];
+    this.isDisabled = true;
+
+    this.init();
+  }
+
+  init() {
+    this.inputs = this.form.querySelectorAll('input, textarea, select');
+    this.submits = this.form.querySelectorAll('input[type=submit], button[type=submit]');
+
+    this.inputs.forEach(DirtySubmitForm.initInput);
+    this.toggleSubmission();
+
+    this.registerListeners();
+  }
+
+  registerListeners() {
+    const throttledUpdateDirtyInput = _.throttle(
+      event => this.updateDirtyInput(event),
+      DirtySubmitForm.THROTTLE_DURATION,
+    );
+    this.form.addEventListener('input', throttledUpdateDirtyInput);
+    this.form.addEventListener('submit', event => this.formSubmit(event));
+  }
+
+  updateDirtyInput(event) {
+    const input = event.target;
+
+    if (!input.dataset.dirtySubmitOriginalValue) return;
+
+    this.updateDirtyInputs(input);
+    this.toggleSubmission();
+  }
+
+  updateDirtyInputs(input) {
+    const { name } = input;
+    const isDirty =
+      input.dataset.dirtySubmitOriginalValue !== DirtySubmitForm.inputCurrentValue(input);
+    const indexOfInputName = this.dirtyInputs.indexOf(name);
+    const isExisting = indexOfInputName !== -1;
+
+    if (isDirty && !isExisting) this.dirtyInputs.push(name);
+    if (!isDirty && isExisting) this.dirtyInputs.splice(indexOfInputName, 1);
+  }
+
+  toggleSubmission() {
+    this.isDisabled = this.dirtyInputs.length === 0;
+    this.submits.forEach(element => {
+      element.disabled = this.isDisabled;
+    });
+  }
+
+  formSubmit(event) {
+    if (this.isDisabled) {
+      event.preventDefault();
+      event.stopImmediatePropagation();
+    }
+
+    return !this.isDisabled;
+  }
+
+  static initInput(element) {
+    element.dataset.dirtySubmitOriginalValue = DirtySubmitForm.inputCurrentValue(element);
+  }
+
+  static isInputCheckable(input) {
+    return input.type === 'checkbox' || input.type === 'radio';
+  }
+
+  static inputCurrentValue(input) {
+    return DirtySubmitForm.isInputCheckable(input) ? input.checked.toString() : input.value;
+  }
+}
+
+DirtySubmitForm.THROTTLE_DURATION = 500;
+
+export default DirtySubmitForm;

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

@@ -2,6 +2,7 @@ import groupAvatar from '~/group_avatar';
 import TransferDropdown from '~/groups/transfer_dropdown';
 import initConfirmDangerModal from '~/confirm_danger_modal';
 import initSettingsPanels from '~/settings_panels';
+import dirtySubmitFactory from '~/dirty_submit/dirty_submit_factory';
 import mountBadgeSettings from '~/pages/shared/mount_badge_settings';
 import { GROUP_BADGE } from '~/badges/constants';
 
@@ -10,5 +11,8 @@ document.addEventListener('DOMContentLoaded', () => {
   new TransferDropdown(); // eslint-disable-line no-new
   initConfirmDangerModal();
   initSettingsPanels();
+  dirtySubmitFactory(
+    document.querySelectorAll('.js-general-settings-form, .js-general-permissions-form'),
+  );
   mountBadgeSettings(GROUP_BADGE);
 });

app/assets/javascripts/settings_panels.js

 import $ from 'jquery';
+import { __ } from './locale';
 
 function expandSection($section) {
-  $section.find('.js-settings-toggle').text('Collapse');
+  $section.find('.js-settings-toggle:not(.js-settings-toggle-trigger-only)').text(__('Collapse'));
   $section.find('.settings-content').off('scroll.expandSection').scrollTop(0);
   $section.addClass('expanded');
   if (!$section.hasClass('no-animate')) {
@@ -11,7 +12,7 @@ function expandSection($section) {
 }
 
 function closeSection($section) {
-  $section.find('.js-settings-toggle').text('Expand');
+  $section.find('.js-settings-toggle:not(.js-settings-toggle-trigger-only)').text(__('Expand'));
   $section.find('.settings-content').on('scroll.expandSection', () => expandSection($section));
   $section.removeClass('expanded');
   if (!$section.hasClass('no-animate')) {

app/assets/stylesheets/pages/settings.scss

@@ -42,6 +42,10 @@
     margin-top: 0;
   }
 
+  .settings-title {
+    cursor: pointer;
+  }
+
   button {
     position: absolute;
     top: 20px;

app/views/admin/groups/_form.html.haml

@@ -2,7 +2,7 @@
   = form_errors(@group)
   = render 'shared/group_form', f: f
 
-  = render_if_exists 'shared/repository_size_limit_setting', form: f, type: :group
+  = render_if_exists 'shared/old_repository_size_limit_setting', form: f, type: :group
   = render_if_exists 'admin/namespace_plan', f: f
 
   .form-group.row.group-description-holder
@@ -10,11 +10,11 @@
     .col-sm-10
       = render 'shared/choose_group_avatar_button', f: f
 
-  = render 'shared/visibility_level', f: f, visibility_level: visibility_level, can_change_visibility_level: can_change_group_visibility_level?(@group), form_model: @group
+  = render 'shared/old_visibility_level', f: f, visibility_level: visibility_level, can_change_visibility_level: can_change_group_visibility_level?(@group), form_model: @group, with_label: false
 
   .form-group.row
     .offset-sm-2.col-sm-10
-      = render 'shared/allow_request_access', form: f
+      = render 'shared/allow_request_access', form: f, bold_label: true
 
   = render 'groups/group_admin_settings', f: f
 

app/views/groups/_group_admin_settings.html.haml

@@ -10,7 +10,7 @@
         %br/
         %span.descr This setting can be overridden in each project.
 
-= render partial: 'groups/ee/project_creation_level', locals: { form: f, group: @group }
+= render partial: 'groups/ee/old_project_creation_level', locals: { form: f, group: @group }
 
 .form-group.row
   = f.label :require_two_factor_authentication, 'Two-factor authentication', class: 'col-form-label col-sm-2 pt-0'

app/views/groups/edit.html.haml

@@ -3,31 +3,31 @@
 - expanded = Rails.env.test?
 
 
-%section.settings.gs-general.no-animate#js-general-settings{ class: ('expanded' if expanded) }
+%section.settings.gs-general.no-animate#js-general-settings{ class: ('expanded') }
   .settings-header
-    %h4
-      = _('General')
+    %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
+      = _('Naming, visibility')
     %button.btn.js-settings-toggle{ type: 'button' }
-      = expanded ? _('Collapse') : _('Expand')
+      = _('Collapse')
     %p
-      = _('Update your group name, description, avatar, and other general settings.')
+      = _('Update your group name, description, avatar, and visibility.')
   .settings-content
     = render 'groups/settings/general'
 
 %section.settings.gs-permissions.no-animate#js-permissions-settings{ class: ('expanded' if expanded) }
   .settings-header
-    %h4
-      = _('Permissions')
+    %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
+      = _('Permissions, LFS, 2FA')
     %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? _('Collapse') : _('Expand')
     %p
-      = _('Enable or disable certain group features and choose access levels.')
+      = _('Advanced permissions, Large File Storage and Two-Factor authentication settings.')
   .settings-content
     = render 'groups/settings/permissions'
 
-%section.settings.no-animate{ class: ('expanded' if expanded) }
+%section.settings.no-animate#js-badge-settings{ class: ('expanded' if expanded) }
   .settings-header
-    %h4
+    %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
       = s_('GroupSettings|Badges')
     %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? 'Collapse' : 'Expand'
@@ -39,8 +39,8 @@
 
 %section.settings.gs-advanced.no-animate#js-advanced-settings{ class: ('expanded' if expanded) }
   .settings-header
-    %h4
-      = _('Advanced')
+    %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
+      = _('Path, transfer, remove')
     %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? _('Collapse') : _('Expand')
     %p

app/views/groups/new.html.haml

@@ -27,7 +27,7 @@
         .col-sm-10
           = render 'shared/choose_group_avatar_button', f: f
 
-      = render 'shared/visibility_level', f: f, visibility_level: default_group_visibility, can_change_visibility_level: true, form_model: @group
+      = render 'shared/old_visibility_level', f: f, visibility_level: @group.visibility_level, can_change_visibility_level: can_change_group_visibility_level?(@group), form_model: @group, with_label: false
 
       = render 'create_chat_team', f: f if Gitlab.config.mattermost.enabled
 

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

@@ -23,16 +23,6 @@
 
     = 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
@@ -47,3 +37,13 @@
         %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'
+
+.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) }

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

-= form_for @group, html: { multipart: true, class: 'gl-show-field-errors' }, authenticity_token: true do |f|
+= form_for @group, html: { multipart: true, class: 'gl-show-field-errors js-general-settings-form' }, authenticity_token: true do |f|
   %input{ type: 'hidden', name: 'update_section', value: 'js-general-settings' }
   = form_errors(@group)
 
   %fieldset
     .row
-      .form-group.col-md-9
-        = f.label :name, class: 'label-bold' do
-          Group name
+      .form-group.col-md-5
+        = f.label :name, _('Group name'), class: 'label-bold'
         = f.text_field :name, class: 'form-control'
 
-      .form-group.col-md-3
-        = f.label :id, class: 'label-bold' do
-          Group ID
-        = f.text_field :id, class: 'form-control', readonly: true
+      .form-group.col-md-7
+        = f.label :id, _('Group ID'), class: 'label-bold'
+        = f.text_field :id, class: 'form-control w-auto', readonly: true
+
+    .row.prepend-top-8
+      .form-group.col-md-9
+        = f.label :description, _('Group description (optional)'), class: 'label-bold'
+        = f.text_area :description, class: 'form-control', rows: 3, maxlength: 250
 
-    .form-group
-      = f.label :description, class: 'label-bold' do
-        Group description
-        %span.light (optional)
-      = f.text_area :description, class: 'form-control', rows: 3, maxlength: 250
+  .row= render_if_exists 'shared/repository_size_limit_setting', form: f, type: :group
 
-    = render_if_exists 'shared/repository_size_limit_setting', form: f, type: :group
+  .form-group.prepend-top-default.append-bottom-20
+    .avatar-container.s90
+      = group_icon(@group, alt: '', class: 'avatar group-avatar s90')
+    = f.label :avatar, _('Group avatar'), class: 'label-bold d-block'
+    = 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'
 
-    .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'
+  = render 'shared/visibility_level', f: f, visibility_level: @group.visibility_level, can_change_visibility_level: can_change_group_visibility_level?(@group), form_model: @group
 
-  = f.submit 'Save group', class: 'btn btn-success'
+  = f.submit _('Save changes'), class: 'btn btn-success mt-4 js-dirty-submit'

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

+- docs_link_url = help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs')
+- docs_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: docs_link_url }
+
+%h5= _('Large File Storage')
+
+%p= s_('Check the %{docs_link_start}documentation%{docs_link_end}.').html_safe % { docs_link_start: docs_link_start, docs_link_end: '</a>'.html_safe }
+
+.form-group.append-bottom-default
+  .form-check
+    = f.check_box :lfs_enabled, checked: @group.lfs_enabled?, class: 'form-check-input'
+    = f.label :lfs_enabled, class: 'form-check-label' do
+      %span
+        = _('Allow projects within this group to use Git LFS')
+      %br/
+      %span.text-muted= _('This setting can be overridden in each project.')

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

-= form_for @group, html: { multipart: true, class: 'gl-show-field-errors' }, authenticity_token: true do |f|
+= form_for @group, html: { multipart: true, class: 'gl-show-field-errors js-general-permissions-form' }, authenticity_token: true do |f|
   %input{ type: 'hidden', name: 'update_section', value: 'js-permissions-settings' }
   = 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
+    %h5= _('Permissions')
+    .form-group
+      = render 'shared/allow_request_access', form: f
 
-    .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.pt-0
-        = s_('GroupSettings|Share with group lock')
-      .col-sm-10
-        .form-check
-          = f.check_box :share_with_group_lock, disabled: !can_change_share_with_group_lock?(@group), class: 'form-check-input'
-          = f.label :share_with_group_lock, class: 'form-check-label' do
-            %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
+    .form-group.append-bottom-default
+      .form-check
+        = f.check_box :share_with_group_lock, disabled: !can_change_share_with_group_lock?(@group), class: 'form-check-input'
+        = f.label :share_with_group_lock, class: 'form-check-label' do
+          %span
+            - 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.text-muted= share_with_group_lock_help_text(@group)
 
+    = render 'groups/settings/lfs', f: f
+    = render partial: 'groups/ee/project_creation_level', locals: { form: f, group: @group }
+    = render 'groups/settings/two_factor_auth', f: f
     = render_if_exists 'groups/member_lock_setting', f: f, group: @group
 
-  = f.submit 'Save group', class: 'btn btn-success'
+  = f.submit _('Save changes'), class: 'btn btn-success prepend-top-default js-dirty-submit'

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

+- docs_link_url = help_page_path('security/two_factor_authentication', anchor: 'enforcing-2fa-for-all-users-in-a-group')
+- docs_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: docs_link_url }
+
+%h5= _('Two-factor authentication')
+
+%p= s_('Check the %{docs_link_start}documentation%{docs_link_end}.').html_safe % { docs_link_start: docs_link_start, docs_link_end: '</a>'.html_safe }
+
+.form-group
+  .form-check
+    = f.check_box :require_two_factor_authentication, class: 'form-check-input'
+    = f.label :require_two_factor_authentication, class: 'form-check-label' do
+      %span= _('Require all users in this group to setup Two-factor authentication')
+.form-group
+  = f.label :two_factor_grace_period, _('Time before enforced'), class: 'label-bold'
+  = f.text_field :two_factor_grace_period, class: 'form-control form-control-sm w-auto'
+  .form-text.text-muted= _('Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication')

app/views/shared/_allow_request_access.html.haml

+- label_class = local_assigns.fetch(:bold_label, false) ? 'font-weight-bold' : ''
+
 .form-check
   = form.check_box :request_access_enabled, class: 'form-check-input'
   = form.label :request_access_enabled, class: 'form-check-label' do
-    %strong Allow users to request access
+    %span{ class: label_class }= _('Allow users to request access')
     %br
-    %span.descr Allow users to request access if visibility is public or internal.
+    %span.text-muted= _('Allow users to request access if visibility is public or internal.')

app/views/shared/_old_visibility_level.html.haml

+.form-group.row
+  .col-sm-2.col-form-label
+    = _('Visibility level')
+    = link_to icon('question-circle'), help_page_path("public_access/public_access")
+  .col-sm-10
+    = render 'shared/visibility_level', f: f, visibility_level: visibility_level, can_change_visibility_level: can_change_visibility_level, form_model: form_model, with_label: with_label

app/views/shared/_visibility_level.html.haml

 - with_label = local_assigns.fetch(:with_label, true)
 
-.form-group.row.visibility-level-setting
+.form-group.visibility-level-setting
   - if with_label
-    = f.label :visibility_level, class: 'col-form-label col-sm-2 pt-0' do
-      Visibility Level
-      = link_to icon('question-circle'), help_page_path("public_access/public_access")
-  %div{ :class => (with_label ? "col-sm-10" : "col-sm-12") }
-    - if can_change_visibility_level
-      = render('shared/visibility_radios', model_method: :visibility_level, form: f, selected_level: visibility_level, form_model: form_model)
-    - else
-      %div
-        %span.info
-          = visibility_level_icon(visibility_level)
-          %strong
-            = visibility_level_label(visibility_level)
-          .light= visibility_level_description(visibility_level, form_model)
+    = f.label :visibility_level, _('Visibility level'), class: 'label-bold append-bottom-0'
+    %p
+      = _('Who can see this group?')
+      - visibility_docs_path = help_page_path('public_access/public_access')
+      - docs_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: visibility_docs_path }
+      = s_('Check the %{docs_link_start}documentation%{docs_link_end}.').html_safe % { docs_link_start: docs_link_start, docs_link_end: '</a>'.html_safe }
+  - if can_change_visibility_level
+    = render('shared/visibility_radios', model_method: :visibility_level, form: f, selected_level: visibility_level, form_model: form_model)
+  - else
+    %div
+      %span.info
+        = visibility_level_icon(visibility_level)
+        %strong
+          = visibility_level_label(visibility_level)
+        .light= visibility_level_description(visibility_level, form_model)

app/views/shared/snippets/_form.html.haml

@@ -14,7 +14,7 @@
 
     = render 'shared/form_elements/description', model: @snippet, project: @project, form: f
 
-    = render 'shared/visibility_level', f: f, visibility_level: @snippet.visibility_level, can_change_visibility_level: true, form_model: @snippet
+    = render 'shared/old_visibility_level', f: f, visibility_level: @snippet.visibility_level, can_change_visibility_level: true, form_model: @snippet, with_label: false
 
     .file-editor
       .form-group.row

changelogs/unreleased/49417-improve-settings-pages-design-by-prioritizing-content-group-settings.yml

+---
+title: Update group settings/edit page to new design
+merge_request: 21115
+author:
+type: other

doc/development/new_fe_guide/index.md

@@ -19,6 +19,10 @@ Guidance on topics related to development.
 
 Learn about all the dependencies that make up our frontend, including some of our own custom built libraries.
 
+## [Modules](modules/index.md)
+
+Learn about all the internal JavaScript modules that make up our frontend.
+
 ## [Style guides](style/index.md)
 
 Style guides to keep our code consistent.

doc/development/new_fe_guide/modules/dirty_submit.md

+# Dirty Submit
+
+> [Introduced][ce-21115] in GitLab 11.3.  
+> [dirty_submit][dirty-submit]
+
+## Summary
+
+Prevent submitting forms with no changes.
+
+Currently handles `input`, `textarea` and `select` elements.
+
+## Usage
+
+```js
+import dirtySubmitFactory from './dirty_submit/dirty_submit_form';
+
+new DirtySubmitForm(document.querySelector('form'));
+// or
+new DirtySubmitForm(document.querySelectorAll('form'));
+```
+
+[ce-21115]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21115
+[dirty-submit]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/dirty_submit/
\ No newline at end of file

doc/development/new_fe_guide/modules/index.md

+# Modules
+
+* [DirtySubmit](dirty_submit.md)
+
+  Disable form submits until there are unsaved changes.
\ No newline at end of file

ee/app/views/groups/_member_lock_setting.html.haml

 - return unless can?(current_user, :admin_group, group) && License.feature_available?(:member_lock)
 
-%hr
-.form-group.row
-  = f.label :membership_lock, class: 'col-form-label col-sm-2' do
-    Member lock
-  .col-sm-10
-    .form-check
-      = f.check_box :membership_lock, class: 'form-check-input'
-      %span.descr Prevent adding new members to project membership within this group
+%h5= _('Member lock')
+.form-group
+  .form-check
+    = f.check_box :membership_lock, class: 'form-check-input'
+    = f.label :membership_lock, class: 'form-check-label' do
+      %span= _('Prevent adding new members to project membership within this group')

ee/app/views/groups/ee/_old_project_creation_level.html.haml

+- return unless group.feature_available?(:project_creation_level)
+- form = local_assigns.fetch(:form)
+- group = local_assigns.fetch(:group)
+.form-group.row
+  = form.label s_('ProjectCreationLevel|Allowed to create projects'), class: 'col-form-label col-sm-2'
+  .col-sm-10
+    = form.select :project_creation_level, options_for_select(::Gitlab::Access.project_creation_options, group.project_creation_level), {}, class: 'form-control'

ee/app/views/groups/ee/_project_creation_level.html.haml

 - return unless group.feature_available?(:project_creation_level)
 - form = local_assigns.fetch(:form)
 - group = local_assigns.fetch(:group)
-.form-group.row
-  = form.label s_('ProjectCreationLevel|Allowed to create projects'), class: 'col-form-label col-sm-2'
-  .col-sm-10
-    = form.select :project_creation_level, options_for_select(::Gitlab::Access.project_creation_options, group.project_creation_level), {}, class: 'form-control'
+
+.form-group.col-md-9.row.prepend-top-8
+  = form.label :description, s_('ProjectCreationLevel|Allowed to create projects'), class: 'label-bold'
+  = form.select :project_creation_level, options_for_select(::Gitlab::Access.project_creation_options, group.project_creation_level), {}, class: 'form-control'

ee/app/views/shared/_old_repository_size_limit_setting.html.haml

+- return unless current_user.admin? && License.feature_available?(:repository_size_limit)
+
+- form = local_assigns.fetch(:form)
+- type = local_assigns.fetch(:type)
+- label_class = (type == :project) ? 'label-bold' : 'col-form-label col-sm-2'
+
+.form-group.row
+  = form.label :repository_size_limit, class: label_class do
+    Repository size limit (MB)
+  - if type == :project
+    = form.number_field :repository_size_limit, value: form.object.repository_size_limit.try(:to_mb), class: 'form-control', min: 0
+    %span.form-text.text-muted#repository_size_limit_help_block
+      = size_limit_message(@project)
+  - elsif type == :group
+    .col-sm-10
+      = form.number_field :repository_size_limit, value: form.object.repository_size_limit.try(:to_mb), class: 'form-control', min: 0
+      %span.form-text.text-muted#repository_size_limit_help_block
+        = size_limit_message_for_group(@group)

ee/app/views/shared/_repository_size_limit_setting.html.haml

@@ -2,17 +2,11 @@
 
 - form = local_assigns.fetch(:form)
 - type = local_assigns.fetch(:type)
-- label_class = (type == :project) ? 'label-bold' : 'col-form-label col-sm-2'
+- form_group_class = type === :group ? 'col-md-9' : ''
 
-.form-group.row
-  = form.label :repository_size_limit, class: label_class do
+.form-group{ class: form_group_class }
+  = form.label :repository_size_limit, class: 'label-bold' do
     Repository size limit (MB)
-  - if type == :project
-    = form.number_field :repository_size_limit, value: form.object.repository_size_limit.try(:to_mb), class: 'form-control', min: 0
-    %span.form-text.text-muted#repository_size_limit_help_block
-      = size_limit_message(@project)
-  - elsif type == :group
-    .col-sm-10
-      = form.number_field :repository_size_limit, value: form.object.repository_size_limit.try(:to_mb), class: 'form-control', min: 0
-      %span.form-text.text-muted#repository_size_limit_help_block
-        = size_limit_message_for_group(@group)
+  = form.number_field :repository_size_limit, value: form.object.repository_size_limit.try(:to_mb), class: 'form-control', min: 0
+  %span.form-text.text-muted#repository_size_limit_help_block
+    = type === :project ? size_limit_message(@project) : size_limit_message_for_group(@group)

locale/gitlab.pot

@@ -490,7 +490,7 @@ msgstr ""
 msgid "AdminUsers|To confirm, type %{username}"
 msgstr ""
 
-msgid "Advanced"
+msgid "Advanced permissions, Large File Storage and Two-Factor authentication settings."
 msgstr ""
 
 msgid "Advanced settings"
@@ -511,6 +511,9 @@ msgstr ""
 msgid "Allow commits from members who can merge to the target branch."
 msgstr ""
 
+msgid "Allow projects within this group to use Git LFS"
+msgstr ""
+
 msgid "Allow public access to pipelines and job details, including output logs and artifacts"
 msgstr ""
 
@@ -520,6 +523,12 @@ msgstr ""
 msgid "Allow requests to the local network from hooks and services."
 msgstr ""
 
+msgid "Allow users to request access"
+msgstr ""
+
+msgid "Allow users to request access if visibility is public or internal."
+msgstr ""
+
 msgid "Allows you to add and manage Kubernetes clusters."
 msgstr ""
 
@@ -535,6 +544,9 @@ msgstr ""
 msgid "Alternatively, you can use a %{personal_access_token_link}. When you create your Personal Access Token, you will need to select the <code>repo</code> scope, so we can display a list of your public and private repositories which are available to import."
 msgstr ""
 
+msgid "Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication"
+msgstr ""
+
 msgid "An SSH key will be automatically generated when the form is submitted. For more information, please refer to the documentation."
 msgstr ""
 
@@ -1395,6 +1407,9 @@ msgstr ""
 msgid "Chat"
 msgstr ""
 
+msgid "Check the %{docs_link_start}documentation%{docs_link_end}."
+msgstr ""
+
 msgid "Checking %{text} availability…"
 msgstr ""
 
@@ -2886,9 +2901,6 @@ msgstr ""
 msgid "Enable group Runners"
 msgstr ""
 
-msgid "Enable or disable certain group features and choose access levels."
-msgstr ""
-
 msgid "Enable or disable the Pseudonymizer data collection."
 msgstr ""
 
@@ -3879,6 +3891,9 @@ msgstr ""
 msgid "Group avatar"
 msgstr ""
 
+msgid "Group description (optional)"
+msgstr ""
+
 msgid "Group details"
 msgstr ""
 
@@ -3888,6 +3903,9 @@ msgstr ""
 msgid "Group maintainers can register group runners in the %{link}"
 msgstr ""
 
+msgid "Group name"
+msgstr ""
+
 msgid "Group: %{group_name}"
 msgstr ""
 
@@ -3939,9 +3957,6 @@ msgstr ""
 msgid "GroupSettings|Prevent sharing a project within %{group} with other groups"
 msgstr ""
 
-msgid "GroupSettings|Share with group lock"
-msgstr ""
-
 msgid "GroupSettings|This setting is applied on %{ancestor_group} and has been overridden on this subgroup."
 msgstr ""
 
@@ -4480,6 +4495,9 @@ msgstr ""
 msgid "Labels|Promoting %{labelTitle} will make it available for all projects inside %{groupName}. Existing project labels with the same title will be merged. This action cannot be reversed."
 msgstr ""
 
+msgid "Large File Storage"
+msgstr ""
+
 msgid "Last %d day"
 msgid_plural "Last %d days"
 msgstr[0] ""
@@ -4802,6 +4820,9 @@ msgstr ""
 msgid "Median"
 msgstr ""
 
+msgid "Member lock"
+msgstr ""
+
 msgid "Member since %{date}"
 msgstr ""
 
@@ -5108,6 +5129,9 @@ msgstr ""
 msgid "Name:"
 msgstr ""
 
+msgid "Naming, visibility"
+msgstr ""
+
 msgid "Nav|Help"
 msgstr ""
 
@@ -5553,6 +5577,9 @@ msgstr ""
 msgid "Paste your public SSH key, which is usually contained in the file '~/.ssh/id_rsa.pub' and begins with 'ssh-rsa'. Don't use your private SSH key."
 msgstr ""
 
+msgid "Path, transfer, remove"
+msgstr ""
+
 msgid "Path:"
 msgstr ""
 
@@ -5580,6 +5607,9 @@ msgstr ""
 msgid "Permissions"
 msgstr ""
 
+msgid "Permissions, LFS, 2FA"
+msgstr ""
+
 msgid "Personal Access Token"
 msgstr ""
 
@@ -5790,6 +5820,9 @@ msgstr ""
 msgid "Press Enter or click to search"
 msgstr ""
 
+msgid "Prevent adding new members to project membership within this group"
+msgstr ""
+
 msgid "Preview"
 msgstr ""
 
@@ -6488,12 +6521,21 @@ msgstr ""
 msgid "Remove avatar"
 msgstr ""
 
+msgid "Remove group"
+msgstr ""
+
 msgid "Remove priority"
 msgstr ""
 
 msgid "Remove project"
 msgstr ""
 
+msgid "Removed group can not be restored!"
+msgstr ""
+
+msgid "Removing group will cause all child projects and resources to be removed."
+msgstr ""
+
 msgid "Rename"
 msgstr ""
 
@@ -6593,6 +6635,9 @@ msgstr ""
 msgid "Requests Profiles"
 msgstr ""
 
+msgid "Require all users in this group to setup Two-factor authentication"
+msgstr ""
+
 msgid "Require all users to accept Terms of Service and Privacy Policy when they access GitLab."
 msgstr ""
 
@@ -7841,6 +7886,9 @@ msgstr ""
 msgid "This runner will only run on pipelines triggered on protected branches"
 msgstr ""
 
+msgid "This setting can be overridden in each project."
+msgstr ""
+
 msgid "This source diff could not be displayed because it is too large."
 msgstr ""
 
@@ -7868,6 +7916,9 @@ msgstr ""
 msgid "Time before an issue starts implementation"
 msgstr ""
 
+msgid "Time before enforced"
+msgstr ""
+
 msgid "Time between merge request creation and merge/close"
 msgstr ""
 
@@ -8193,6 +8244,9 @@ msgstr ""
 msgid "Twitter"
 msgstr ""
 
+msgid "Two-factor authentication"
+msgstr ""
+
 msgid "Type"
 msgstr ""
 
@@ -8268,7 +8322,7 @@ msgstr ""
 msgid "Update now"
 msgstr ""
 
-msgid "Update your group name, description, avatar, and other general settings."
+msgid "Update your group name, description, avatar, and visibility."
 msgstr ""
 
 msgid "Updating"
@@ -8454,6 +8508,9 @@ msgstr ""
 msgid "Visibility and access controls"
 msgstr ""
 
+msgid "Visibility level"
+msgstr ""
+
 msgid "Visibility level:"
 msgstr ""
 
@@ -8511,6 +8568,9 @@ msgstr ""
 msgid "When leaving the URL blank, classification labels can still be specified without disabling cross project features or performing external authorization checks."
 msgstr ""
 
+msgid "Who can see this group?"
+msgstr ""
+
 msgid "Wiki"
 msgstr ""
 

spec/features/groups/group_settings_spec.rb

@@ -125,7 +125,7 @@ describe 'Edit group settings' do
 
   def save_group
     page.within('.gs-general') do
-      click_button 'Save group'
+      click_button 'Save changes'
     end
   end
 end

spec/features/groups/share_lock_spec.rb

@@ -60,14 +60,14 @@ describe 'Group share with group lock' do
   def enable_group_lock
     page.within('.gs-permissions') do
       check 'group_share_with_group_lock'
-      click_on 'Save group'
+      click_on 'Save changes'
     end
   end
 
   def disable_group_lock
     page.within('.gs-permissions') do
       uncheck 'group_share_with_group_lock'
-      click_on 'Save group'
+      click_on 'Save changes'
     end
   end
 end

spec/features/groups_spec.rb

@@ -140,10 +140,13 @@ describe 'Group' do
       visit path
     end
 
+    it_behaves_like 'dirty submit form', [{ form: '.js-general-settings-form', input: 'input[name="group[name]"]' },
+                                          { form: '.js-general-permissions-form', input: 'input[name="group[two_factor_grace_period]"]' }]
+
     it 'saves new settings' do
       page.within('.gs-general') do
         fill_in 'group_name', with: new_name
-        click_button 'Save group'
+        click_button 'Save changes'
       end
 
       expect(page).to have_content 'successfully updated'

spec/features/uploads/user_uploads_avatar_to_group_spec.rb

@@ -15,7 +15,7 @@ describe 'User uploads avatar to group' do
     )
 
     page.within('.gs-general') do
-      click_button 'Save group'
+      click_button 'Save changes'
     end
 
     visit group_path(group)

spec/javascripts/dirty_submit/dirty_submit_collection_spec.js

+import DirtySubmitCollection from '~/dirty_submit/dirty_submit_collection';
+import { setInput, createForm } from './helper';
+
+describe('DirtySubmitCollection', () => {
+  it('disables submits until there are changes', done => {
+    const testElementsCollection = [createForm(), createForm()];
+    const forms = testElementsCollection.map(testElements => testElements.form);
+
+    new DirtySubmitCollection(forms); // eslint-disable-line no-new
+
+    testElementsCollection.forEach(testElements => {
+      const { input, submit } = testElements;
+      const originalValue = input.value;
+
+      expect(submit.disabled).toBe(true);
+
+      return setInput(input, `${originalValue} changes`)
+        .then(() => expect(submit.disabled).toBe(false))
+        .then(() => setInput(input, originalValue))
+        .then(() => expect(submit.disabled).toBe(true))
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+});

spec/javascripts/dirty_submit/dirty_submit_factory_spec.js

+import dirtySubmitFactory from '~/dirty_submit/dirty_submit_factory';
+import DirtySubmitForm from '~/dirty_submit/dirty_submit_form';
+import DirtySubmitCollection from '~/dirty_submit/dirty_submit_collection';
+import { createForm } from './helper';
+
+describe('DirtySubmitCollection', () => {
+  it('returns a DirtySubmitForm instance for single form elements', () => {
+    const { form } = createForm();
+
+    expect(dirtySubmitFactory(form) instanceof DirtySubmitForm).toBe(true);
+  });
+
+  it('returns a DirtySubmitCollection instance for a collection of form elements', () => {
+    const forms = [createForm().form, createForm().form];
+
+    expect(dirtySubmitFactory(forms) instanceof DirtySubmitCollection).toBe(true);
+  });
+});

spec/javascripts/dirty_submit/dirty_submit_form_spec.js

+
+import DirtySubmitForm from '~/dirty_submit/dirty_submit_form';
+import { setInput, createForm } from './helper';
+
+describe('DirtySubmitForm', () => {
+  it('disables submit until there are changes', done => {
+    const { form, input, submit } = createForm();
+    const originalValue = input.value;
+
+    new DirtySubmitForm(form); // eslint-disable-line no-new
+
+    expect(submit.disabled).toBe(true);
+
+    return setInput(input, `${originalValue} changes`)
+      .then(() => expect(submit.disabled).toBe(false))
+      .then(() => setInput(input, originalValue))
+      .then(() => expect(submit.disabled).toBe(true))
+      .then(done)
+      .catch(done.fail);
+  });
+});

spec/javascripts/dirty_submit/helper.js

+import DirtySubmitForm from '~/dirty_submit/dirty_submit_form';
+import setTimeoutPromiseHelper from '../helpers/set_timeout_promise_helper';
+
+export function setInput(element, value) {
+  element.value = value;
+
+  element.dispatchEvent(
+    new Event('input', {
+      bubbles: true,
+      cancelable: true,
+    }),
+  );
+
+  return setTimeoutPromiseHelper(DirtySubmitForm.THROTTLE_DURATION);
+}
+
+export function createForm() {
+  const form = document.createElement('form');
+  form.innerHTML = `
+    <input type="text" value="original" class="js-input" name="input" />
+    <button type="submit" class="js-dirty-submit"></button>
+  `;
+  const input = form.querySelector('.js-input');
+  const submit = form.querySelector('.js-dirty-submit');
+
+  return {
+    form,
+    input,
+    submit,
+  };
+}

spec/javascripts/fixtures/groups.rb

@@ -17,6 +17,16 @@ describe 'Groups (JavaScript fixtures)', type: :controller do
     sign_in(admin)
   end
 
+  describe GroupsController, '(JavaScript fixtures)', type: :controller do
+    it 'groups/edit.html.raw' do |example|
+      get :edit,
+        id: group
+
+      expect(response).to be_success
+      store_frontend_fixture(response, example.description)
+    end
+  end
+
   describe Groups::Settings::CiCdController, '(JavaScript fixtures)', type: :controller do
     it 'groups/ci_cd_settings.html.raw' do |example|
       get :show,

spec/javascripts/settings_panels_spec.js

+import $ from 'jquery';
 import initSettingsPanels from '~/settings_panels';
 
 describe('Settings Panels', () => {
-  preloadFixtures('projects/ci_cd_settings.html.raw');
+  preloadFixtures('groups/edit.html.raw');
 
   beforeEach(() => {
-    loadFixtures('projects/ci_cd_settings.html.raw');
+    loadFixtures('groups/edit.html.raw');
   });
 
   describe('initSettingsPane', () => {
@@ -13,17 +14,32 @@ describe('Settings Panels', () => {
     });
 
     it('should expand linked hash fragment panel', () => {
-      window.location.hash = '#autodevops-settings';
+      window.location.hash = '#js-general-settings';
 
-      const pipelineSettingsPanel = document.querySelector('#autodevops-settings');
+      const panel = document.querySelector('#js-general-settings');
       // Our test environment automatically expands everything so we need to clear that out first
-      pipelineSettingsPanel.classList.remove('expanded');
+      panel.classList.remove('expanded');
 
-      expect(pipelineSettingsPanel.classList.contains('expanded')).toBe(false);
+      expect(panel.classList.contains('expanded')).toBe(false);
 
       initSettingsPanels();
 
-      expect(pipelineSettingsPanel.classList.contains('expanded')).toBe(true);
+      expect(panel.classList.contains('expanded')).toBe(true);
     });
   });
+
+  it('does not change the text content of triggers', () => {
+    const panel = document.querySelector('#js-general-settings');
+    const trigger = panel.querySelector('.js-settings-toggle-trigger-only');
+    const originalText = trigger.textContent;
+
+    initSettingsPanels();
+
+    expect(panel.classList.contains('expanded')).toBe(true);
+
+    $(trigger).click();
+
+    expect(panel.classList.contains('expanded')).toBe(false);
+    expect(trigger.textContent).toEqual(originalText);
+  });
 });

spec/support/shared_examples/dirty_submit_form_shared_examples.rb

+shared_examples 'dirty submit form' do |selector_args|
+  selectors = selector_args.is_a?(Array) ? selector_args : [selector_args]
+
+  selectors.each do |selector|
+    it "disables #{selector[:form]} submit until there are changes", :js do
+      form = find(selector[:form])
+      submit = form.first('.js-dirty-submit')
+      input = form.first(selector[:input])
+      original_value = input.value
+
+      expect(submit.disabled?).to be true
+
+      input.set("#{original_value} changes")
+
+      form.find('.js-dirty-submit:not([disabled])', match: :first)
+      expect(submit.disabled?).to be false
+
+      input.set(original_value)
+
+      form.find('.js-dirty-submit[disabled]', match: :first)
+      expect(submit.disabled?).to be true
+    end
+  end
+end