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

5 files changed, 78 insertions, 58 deletions

diff --git a/app/views/groups/settings/_advanced.html.haml b/app/views/groups/settings/_advanced.html.haml
index 3814d45929d..5d211d0e186 100644
--- a/app/views/groups/settings/_advanced.html.haml
+++ b/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) }
diff --git a/app/views/groups/settings/_general.html.haml b/app/views/groups/settings/_general.html.haml
index 0e225fe33a5..0424ece037d 100644
--- a/app/views/groups/settings/_general.html.haml
+++ b/app/views/groups/settings/_general.html.haml
@@ -1,39 +1,33 @@
-= 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
 
-    .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.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'
+    .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'
 
-  = f.submit 'Save group', class: 'btn btn-success'
+    = 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'
diff --git a/app/views/groups/settings/_lfs.html.haml b/app/views/groups/settings/_lfs.html.haml
new file mode 100644
index 00000000000..4674d561c12
--- /dev/null
+++ b/app/views/groups/settings/_lfs.html.haml
@@ -0,0 +1,15 @@
+- 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.')
diff --git a/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml
index 8dc88ec446c..6b0a6e7ed99 100644
--- a/app/views/groups/settings/_permissions.html.haml
+++ b/app/views/groups/settings/_permissions.html.haml
@@ -1,29 +1,24 @@
-= 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 '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'
diff --git a/app/views/groups/settings/_two_factor_auth.html.haml b/app/views/groups/settings/_two_factor_auth.html.haml
new file mode 100644
index 00000000000..5d3f1cbb279
--- /dev/null
+++ b/app/views/groups/settings/_two_factor_auth.html.haml
@@ -0,0 +1,16 @@
+- 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')