Prioritize group settings, improve panel titles, disable submit without changes

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

@@ -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/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
-      = f.label :description, class: 'label-bold' do
-        Group description
-        %span.light (optional)
-      = f.text_area :description, class: 'form-control', rows: 3, maxlength: 250
+      .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.append-bottom-0
+        = f.label :description, _('Group description (optional)'), class: 'label-bold'
+        = 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'
+    .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'
+    = 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 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
-    .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
+    %h5= _('Permissions')
+    .form-group
+      = render 'shared/allow_request_access', form: 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 '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.