diff --git a/app/assets/javascripts/dirty_submit/dirty_submit_collection.js b/app/assets/javascripts/dirty_submit/dirty_submit_collection.js
new file mode 100644
index 00000000000..42b051b2270
--- /dev/null
+++ b/app/assets/javascripts/dirty_submit/dirty_submit_collection.js
@@ -0,0 +1,13 @@
+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;
diff --git a/app/assets/javascripts/dirty_submit/dirty_submit_factory.js b/app/assets/javascripts/dirty_submit/dirty_submit_factory.js
new file mode 100644
index 00000000000..95a896a7f0b
--- /dev/null
+++ b/app/assets/javascripts/dirty_submit/dirty_submit_factory.js
@@ -0,0 +1,9 @@
+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);
+}
diff --git a/app/assets/javascripts/dirty_submit/dirty_submit_form.js b/app/assets/javascripts/dirty_submit/dirty_submit_form.js
new file mode 100644
index 00000000000..5bea47f23c5
--- /dev/null
+++ b/app/assets/javascripts/dirty_submit/dirty_submit_form.js
@@ -0,0 +1,82 @@
+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;
diff --git a/app/assets/javascripts/pages/groups/edit/index.js b/app/assets/javascripts/pages/groups/edit/index.js
index 002b2279fcc..d0bce857029 100644
--- a/app/assets/javascripts/pages/groups/edit/index.js
+++ b/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);
});
diff --git a/app/assets/javascripts/settings_panels.js b/app/assets/javascripts/settings_panels.js
index 37b4a2a4c63..b38421bdf1c 100644
--- a/app/assets/javascripts/settings_panels.js
+++ b/app/assets/javascripts/settings_panels.js
@@ -1,7 +1,8 @@
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')) {
diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss
index dbf8692d69b..ccfa4e00a5b 100644
--- a/app/assets/stylesheets/pages/settings.scss
+++ b/app/assets/stylesheets/pages/settings.scss
@@ -42,6 +42,10 @@
margin-top: 0;
}
+ .settings-title {
+ cursor: pointer;
+ }
+
button {
position: absolute;
top: 20px;
diff --git a/app/views/admin/groups/_form.html.haml b/app/views/admin/groups/_form.html.haml
index 2a117c1414e..5e05568e384 100644
--- a/app/views/admin/groups/_form.html.haml
+++ b/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
diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml
index fc17dd2d310..f3792c5e397 100644
--- a/app/views/groups/edit.html.haml
+++ b/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
diff --git a/app/views/groups/new.html.haml b/app/views/groups/new.html.haml
index 683129fdf6e..86c5f6a7aa3 100644
--- a/app/views/groups/new.html.haml
+++ b/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
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 = ''.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: ''.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 = ''.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: ''.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')
diff --git a/app/views/shared/_allow_request_access.html.haml b/app/views/shared/_allow_request_access.html.haml
index 92268e74b1e..a50f1877d08 100644
--- a/app/views/shared/_allow_request_access.html.haml
+++ b/app/views/shared/_allow_request_access.html.haml
@@ -1,6 +1,8 @@
+- 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.')
diff --git a/app/views/shared/_old_visibility_level.html.haml b/app/views/shared/_old_visibility_level.html.haml
new file mode 100644
index 00000000000..fd576e4fbea
--- /dev/null
+++ b/app/views/shared/_old_visibility_level.html.haml
@@ -0,0 +1,6 @@
+.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
diff --git a/app/views/shared/_visibility_level.html.haml b/app/views/shared/_visibility_level.html.haml
index ba37b37a3b1..2f42a877beb 100644
--- a/app/views/shared/_visibility_level.html.haml
+++ b/app/views/shared/_visibility_level.html.haml
@@ -1,17 +1,19 @@
- 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 = ''.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: ''.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)
diff --git a/app/views/shared/snippets/_form.html.haml b/app/views/shared/snippets/_form.html.haml
index 1b66d3acd40..cf9c3055499 100644
--- a/app/views/shared/snippets/_form.html.haml
+++ b/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
diff --git a/changelogs/unreleased/49417-improve-settings-pages-design-by-prioritizing-content-group-settings.yml b/changelogs/unreleased/49417-improve-settings-pages-design-by-prioritizing-content-group-settings.yml
new file mode 100644
index 00000000000..8ded24a1cd0
--- /dev/null
+++ b/changelogs/unreleased/49417-improve-settings-pages-design-by-prioritizing-content-group-settings.yml
@@ -0,0 +1,5 @@
+---
+title: Update group settings/edit page to new design
+merge_request: 21115
+author:
+type: other
diff --git a/doc/development/new_fe_guide/index.md b/doc/development/new_fe_guide/index.md
index 78931defa24..bfcca9cec7b 100644
--- a/doc/development/new_fe_guide/index.md
+++ b/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.
diff --git a/doc/development/new_fe_guide/modules/dirty_submit.md b/doc/development/new_fe_guide/modules/dirty_submit.md
new file mode 100644
index 00000000000..6c03958b463
--- /dev/null
+++ b/doc/development/new_fe_guide/modules/dirty_submit.md
@@ -0,0 +1,23 @@
+# 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
diff --git a/doc/development/new_fe_guide/modules/index.md b/doc/development/new_fe_guide/modules/index.md
new file mode 100644
index 00000000000..0a7f2dbd819
--- /dev/null
+++ b/doc/development/new_fe_guide/modules/index.md
@@ -0,0 +1,5 @@
+# Modules
+
+* [DirtySubmit](dirty_submit.md)
+
+ Disable form submits until there are unsaved changes.
\ No newline at end of file
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index c1fdad64229..fa88713f5b3 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -423,7 +423,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"
@@ -444,6 +444,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 ""
@@ -453,12 +456,21 @@ 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 ""
msgid "Alternatively, you can use a %{personal_access_token_link}. When you create your Personal Access Token, you will need to select the repo
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 application called %{link_to_client} is requesting access to your GitLab account."
msgstr ""
@@ -1145,6 +1157,9 @@ msgstr ""
msgid "Chat"
msgstr ""
+msgid "Check the %{docs_link_start}documentation%{docs_link_end}."
+msgstr ""
+
msgid "Checking %{text} availability…"
msgstr ""
@@ -2474,9 +2489,6 @@ msgstr ""
msgid "Enable group Runners"
msgstr ""
-msgid "Enable or disable certain group features and choose access levels."
-msgstr ""
-
msgid "Enable or disable version check and usage ping."
msgstr ""
@@ -2984,6 +2996,9 @@ msgstr ""
msgid "Group avatar"
msgstr ""
+msgid "Group description (optional)"
+msgstr ""
+
msgid "Group details"
msgstr ""
@@ -2993,6 +3008,9 @@ msgstr ""
msgid "Group maintainers can register group runners in the %{link}"
msgstr ""
+msgid "Group name"
+msgstr ""
+
msgid "Group: %{group_name}"
msgstr ""
@@ -3008,9 +3026,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 ""
@@ -3496,6 +3511,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] ""
@@ -3905,6 +3923,9 @@ msgstr ""
msgid "Name:"
msgstr ""
+msgid "Naming, visibility"
+msgstr ""
+
msgid "Nav|Help"
msgstr ""
@@ -4290,6 +4311,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 ""
@@ -4317,6 +4341,9 @@ msgstr ""
msgid "Permissions"
msgstr ""
+msgid "Permissions, LFS, 2FA"
+msgstr ""
+
msgid "Personal Access Token"
msgstr ""
@@ -5051,12 +5078,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 ""
@@ -5123,6 +5159,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 ""
@@ -6179,6 +6218,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 ""
@@ -6197,6 +6239,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 ""
@@ -6480,6 +6525,9 @@ msgstr ""
msgid "Twitter"
msgstr ""
+msgid "Two-factor authentication"
+msgstr ""
+
msgid "Type"
msgstr ""
@@ -6546,7 +6594,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"
@@ -6702,6 +6750,9 @@ msgstr ""
msgid "Visibility and access controls"
msgstr ""
+msgid "Visibility level"
+msgstr ""
+
msgid "Visibility level:"
msgstr ""
@@ -6744,6 +6795,9 @@ msgstr ""
msgid "When enabled, users cannot use GitLab until the terms have been accepted."
msgstr ""
+msgid "Who can see this group?"
+msgstr ""
+
msgid "Wiki"
msgstr ""
diff --git a/spec/features/groups/group_settings_spec.rb b/spec/features/groups/group_settings_spec.rb
index 08fd9f8af2a..2cdbdcffbc3 100644
--- a/spec/features/groups/group_settings_spec.rb
+++ b/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
diff --git a/spec/features/groups/share_lock_spec.rb b/spec/features/groups/share_lock_spec.rb
index 5bbe77019ca..704d9f12888 100644
--- a/spec/features/groups/share_lock_spec.rb
+++ b/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
diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb
index e62bd6f8187..63aa26cf5fd 100644
--- a/spec/features/groups_spec.rb
+++ b/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'
diff --git a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
index a07edc42eae..72b53bae46a 100644
--- a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
+++ b/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)
diff --git a/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js b/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js
new file mode 100644
index 00000000000..87a26183b63
--- /dev/null
+++ b/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js
@@ -0,0 +1,25 @@
+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);
+ });
+ });
+});
diff --git a/spec/javascripts/dirty_submit/dirty_submit_factory_spec.js b/spec/javascripts/dirty_submit/dirty_submit_factory_spec.js
new file mode 100644
index 00000000000..40843a68582
--- /dev/null
+++ b/spec/javascripts/dirty_submit/dirty_submit_factory_spec.js
@@ -0,0 +1,18 @@
+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);
+ });
+});
diff --git a/spec/javascripts/dirty_submit/dirty_submit_form_spec.js b/spec/javascripts/dirty_submit/dirty_submit_form_spec.js
new file mode 100644
index 00000000000..86d53fa984a
--- /dev/null
+++ b/spec/javascripts/dirty_submit/dirty_submit_form_spec.js
@@ -0,0 +1,21 @@
+
+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);
+ });
+});
diff --git a/spec/javascripts/dirty_submit/helper.js b/spec/javascripts/dirty_submit/helper.js
new file mode 100644
index 00000000000..6d1e643553c
--- /dev/null
+++ b/spec/javascripts/dirty_submit/helper.js
@@ -0,0 +1,31 @@
+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 = `
+
+
+ `;
+ const input = form.querySelector('.js-input');
+ const submit = form.querySelector('.js-dirty-submit');
+
+ return {
+ form,
+ input,
+ submit,
+ };
+}
diff --git a/spec/javascripts/fixtures/groups.rb b/spec/javascripts/fixtures/groups.rb
index a2035ceae15..b42f442557c 100644
--- a/spec/javascripts/fixtures/groups.rb
+++ b/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,
diff --git a/spec/javascripts/settings_panels_spec.js b/spec/javascripts/settings_panels_spec.js
index c1a69bd7018..3b681a9ff28 100644
--- a/spec/javascripts/settings_panels_spec.js
+++ b/spec/javascripts/settings_panels_spec.js
@@ -1,10 +1,11 @@
+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);
+ });
});
diff --git a/spec/support/shared_examples/dirty_submit_form_shared_examples.rb b/spec/support/shared_examples/dirty_submit_form_shared_examples.rb
new file mode 100644
index 00000000000..ba363593120
--- /dev/null
+++ b/spec/support/shared_examples/dirty_submit_form_shared_examples.rb
@@ -0,0 +1,24 @@
+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