diff options
34 files changed, 494 insertions, 112 deletions
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 = '<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') 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 = '<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) 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 <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 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 = ` + <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, + }; +} 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 |