From 66e8ecffa0990651e7bac46ac65120f5a98b1749 Mon Sep 17 00:00:00 2001 From: Martin Wortschack Date: Tue, 30 Oct 2018 16:23:47 +0000 Subject: Resolve "Create new group: Rename form fields and update UI" --- app/assets/javascripts/group.js | 12 +++--- app/views/groups/new.html.haml | 33 +++++++++------- app/views/shared/_group_form.html.haml | 45 ++++++++++++---------- ...-new-group-rename-form-fields-and-update-ui.yml | 5 +++ config/locales/en.yml | 2 + locale/gitlab.pot | 21 ++++++++++ spec/features/admin/admin_groups_spec.rb | 28 ++++++++++++-- spec/features/dashboard/group_spec.rb | 8 ++-- spec/features/explore/new_menu_spec.rb | 4 +- spec/features/groups_spec.rb | 14 ++++--- spec/services/groups/transfer_service_spec.rb | 2 +- 11 files changed, 117 insertions(+), 57 deletions(-) create mode 100644 changelogs/unreleased/50962-create-new-group-rename-form-fields-and-update-ui.yml diff --git a/app/assets/javascripts/group.js b/app/assets/javascripts/group.js index 4365305c168..903c838e266 100644 --- a/app/assets/javascripts/group.js +++ b/app/assets/javascripts/group.js @@ -1,4 +1,5 @@ import $ from 'jquery'; +import { slugifyWithHyphens } from './lib/utils/text_utility'; export default class Group { constructor() { @@ -7,17 +8,18 @@ export default class Group { this.updateHandler = this.update.bind(this); this.resetHandler = this.reset.bind(this); if (this.groupName.val() === '') { - this.groupPath.on('keyup', this.updateHandler); - this.groupName.on('keydown', this.resetHandler); + this.groupName.on('keyup', this.updateHandler); + this.groupPath.on('keydown', this.resetHandler); } } update() { - this.groupName.val(this.groupPath.val()); + const slug = slugifyWithHyphens(this.groupName.val()); + this.groupPath.val(slug); } reset() { - this.groupPath.off('keyup', this.updateHandler); - this.groupName.off('keydown', this.resetHandler); + this.groupName.off('keyup', this.updateHandler); + this.groupPath.off('keydown', this.resetHandler); } } diff --git a/app/views/groups/new.html.haml b/app/views/groups/new.html.haml index 684b51b8552..0904e44a658 100644 --- a/app/views/groups/new.html.haml +++ b/app/views/groups/new.html.haml @@ -1,12 +1,12 @@ - @hide_breadcrumbs = true - @hide_top_links = true -- page_title 'New Group' -- header_title "Groups", dashboard_groups_path +- page_title _('New Group') +- header_title _("Groups"), dashboard_groups_path +.page-title-holder + %h1.page-title= _('New group') .row.prepend-top-default .col-lg-3.profile-settings-sidebar - %h4.prepend-top-0 - = _('New group') %p - group_docs_path = help_page_path('user/group/index') - group_docs_link_start = ''.html_safe % { url: group_docs_path } @@ -15,24 +15,29 @@ - subgroup_docs_path = help_page_path('user/group/subgroups/index') - subgroup_docs_link_start = ''.html_safe % { url: subgroup_docs_path } = s_('Groups can also be nested by creating %{subgroup_docs_link_start}subgroups%{subgroup_docs_link_end}.').html_safe % { subgroup_docs_link_start: subgroup_docs_link_start, subgroup_docs_link_end: ''.html_safe } + %p + = _('Projects that belong to a group are prefixed with the group namespace. Existing projects may be moved into a group.') .col-lg-9 = form_for @group, html: { class: 'group-form gl-show-field-errors' } do |f| = form_errors(@group) = render 'shared/group_form', f: f, autofocus: true - .form-group.row.group-description-holder - = f.label :avatar, "Group avatar", class: 'col-form-label col-sm-2' - .col-sm-10 - = render 'shared/choose_group_avatar_button', f: f - - = 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 + .row + .form-group.group-description-holder.col-sm-12 + = f.label :avatar, _("Group avatar"), class: 'label-bold' + %div + = render 'shared/choose_group_avatar_button', f: f - = render 'create_chat_team', f: f if Gitlab.config.mattermost.enabled + .form-group.col-sm-12 + %label.label-bold + = _('Visibility level') + %p + = _('Who will be able to see this group?') + = link_to _('View the documentation'), help_page_path("public_access/public_access"), target: '_blank' + = render 'shared/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 - .form-group.row - .offset-sm-2.col-sm-10 - = render 'shared/group_tips' + = render 'create_chat_team', f: f if Gitlab.config.mattermost.enabled .form-actions = f.submit 'Create group', class: "btn btn-success" diff --git a/app/views/shared/_group_form.html.haml b/app/views/shared/_group_form.html.haml index dbed4b94d61..973c756f496 100644 --- a/app/views/shared/_group_form.html.haml +++ b/app/views/shared/_group_form.html.haml @@ -2,10 +2,19 @@ - group_path = root_url - group_path << parent.full_path + '/' if parent -.form-group.row - = f.label :path, class: 'col-form-label col-sm-2' do - Group path - .col-sm-10 +.row + .form-group.group-name-holder.col-sm-12 + = f.label :name, class: 'label-bold' do + = _("Group name") + = f.text_field :name, placeholder: 'My Awesome Group', class: 'form-control input-lg', + required: true, + title: _('Please fill in a descriptive name for your group.'), + autofocus: true + +.row + .form-group.col-xs-12.col-sm-8 + = f.label :path, class: 'label-bold' do + = _("Group URL") .input-group.gl-field-error-anchor .group-root-path.input-group-prepend.has-tooltip{ title: group_path, :'data-placement' => 'bottom' } .input-group-text @@ -13,10 +22,10 @@ - if parent %strong= parent.full_path + '/' = f.hidden_field :parent_id - = f.text_field :path, placeholder: 'open-source', class: 'form-control', + = f.text_field :path, placeholder: 'my-awesome-group', class: 'form-control', autofocus: local_assigns[:autofocus] || false, required: true, pattern: Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX_JS, - title: 'Please choose a group path with no special characters.', + title: _('Please choose a group URL with no special characters.'), "data-bind-in" => "#{'create_chat_team' if Gitlab.config.mattermost.enabled}" - if @group.persisted? @@ -25,23 +34,17 @@ = succeed '.' do = link_to 'Learn more', help_page_path('user/group/index', anchor: 'changing-a-groups-path'), target: '_blank' -.form-group.row.group-name-holder - = f.label :name, class: 'col-form-label col-sm-2' do - Group name - .col-sm-10 - = f.text_field :name, class: 'form-control', - required: true, - title: 'You can choose a descriptive name different from the path.' - - if @group.persisted? - .form-group.row.group-name-holder - = f.label :id, class: 'col-form-label col-sm-2' do - = _("Group ID") - .col-sm-10 + .row + .form-group.group-name-holder.col-sm-8 + = f.label :id, class: 'label-bold' do + = _("Group ID") = f.text_field :id, class: 'form-control', readonly: true -.form-group.row.group-description-holder - = f.label :description, class: 'col-form-label col-sm-2' - .col-sm-10 +.row + .form-group.group-description-holder.col-sm-8 + = f.label :description, class: 'label-bold' do + = _("Group description") + %span (optional) = f.text_area :description, maxlength: 250, class: 'form-control js-gfm-input', rows: 4 diff --git a/changelogs/unreleased/50962-create-new-group-rename-form-fields-and-update-ui.yml b/changelogs/unreleased/50962-create-new-group-rename-form-fields-and-update-ui.yml new file mode 100644 index 00000000000..db374e10c36 --- /dev/null +++ b/changelogs/unreleased/50962-create-new-group-rename-form-fields-and-update-ui.yml @@ -0,0 +1,5 @@ +--- +title: 'Create new group: Rename form fields and update UI' +merge_request: +author: +type: other diff --git a/config/locales/en.yml b/config/locales/en.yml index 795e5d4e6bc..0a43a1d9a6b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -8,6 +8,8 @@ en: issue_link: source: Source issue target: Target issue + group: + path: Group URL errors: messages: label_already_exists_at_group_level: "already exists at group level for %{group}. Please choose another one." diff --git a/locale/gitlab.pot b/locale/gitlab.pot index bb0db059666..18038d84d45 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3014,9 +3014,15 @@ msgstr "" msgid "Group Runners" msgstr "" +msgid "Group URL" +msgstr "" + msgid "Group avatar" msgstr "" +msgid "Group description" +msgstr "" + msgid "Group description (optional)" msgstr "" @@ -4557,12 +4563,18 @@ msgstr "" msgid "Please accept the Terms of Service before continuing." msgstr "" +msgid "Please choose a group URL with no special characters." +msgstr "" + msgid "Please convert them to %{link_to_git}, and go through the %{link_to_import_flow} again." msgstr "" msgid "Please convert them to Git on Google Code, and go through the %{link_to_import_flow} again." msgstr "" +msgid "Please fill in a descriptive name for your group." +msgstr "" + msgid "Please note that this application is not provided by GitLab and you should verify its authenticity before allowing access." msgstr "" @@ -4914,6 +4926,9 @@ msgstr "" msgid "Projects shared with %{group_name}" msgstr "" +msgid "Projects that belong to a group are prefixed with the group namespace. Existing projects may be moved into a group." +msgstr "" + msgid "ProjectsDropdown|Frequently visited" msgstr "" @@ -6804,6 +6819,9 @@ msgstr "" msgid "View replaced file @ " msgstr "" +msgid "View the documentation" +msgstr "" + msgid "Visibility and access controls" msgstr "" @@ -6855,6 +6873,9 @@ msgstr "" msgid "Who can see this group?" msgstr "" +msgid "Who will be able to see this group?" +msgstr "" + msgid "Wiki" msgstr "" diff --git a/spec/features/admin/admin_groups_spec.rb b/spec/features/admin/admin_groups_spec.rb index 96dfde2e08c..735ca60f7da 100644 --- a/spec/features/admin/admin_groups_spec.rb +++ b/spec/features/admin/admin_groups_spec.rb @@ -53,13 +53,33 @@ describe 'Admin Groups' do expect_selected_visibility(internal) end - it 'when entered in group path, it auto filled the group name', :js do + it 'when entered in group name, it auto filled the group path', :js do visit admin_groups_path click_link "New group" - group_path = 'gitlab' + group_name = 'gitlab' + fill_in 'group_name', with: group_name + path_field = find('input#group_path') + expect(path_field.value).to eq group_name + end + + it 'auto populates the group path with the group name', :js do + visit admin_groups_path + click_link "New group" + group_name = 'my gitlab project' + fill_in 'group_name', with: group_name + path_field = find('input#group_path') + expect(path_field.value).to eq 'my-gitlab-project' + end + + it 'when entering in group path, group name does not change anymore', :js do + visit admin_groups_path + click_link "New group" + group_path = 'my-gitlab-project' + group_name = 'My modified gitlab project' fill_in 'group_path', with: group_path - name_field = find('input#group_name') - expect(name_field.value).to eq group_path + fill_in 'group_name', with: group_name + path_field = find('input#group_path') + expect(path_field.value).to eq 'my-gitlab-project' end end diff --git a/spec/features/dashboard/group_spec.rb b/spec/features/dashboard/group_spec.rb index e57fcde8b2c..259f220c68b 100644 --- a/spec/features/dashboard/group_spec.rb +++ b/spec/features/dashboard/group_spec.rb @@ -14,15 +14,15 @@ RSpec.describe 'Dashboard Group' do it 'creates new group', :js do visit dashboard_groups_path find('.btn-success').click - new_path = 'Samurai' + new_name = 'Samurai' new_description = 'Tokugawa Shogunate' - fill_in 'group_path', with: new_path + fill_in 'group_name', with: new_name fill_in 'group_description', with: new_description click_button 'Create group' - expect(current_path).to eq group_path(Group.find_by(name: new_path)) - expect(page).to have_content(new_path) + expect(current_path).to eq group_path(Group.find_by(name: new_name)) + expect(page).to have_content(new_name) expect(page).to have_content(new_description) end end diff --git a/spec/features/explore/new_menu_spec.rb b/spec/features/explore/new_menu_spec.rb index 11f05b6d220..259f22139ef 100644 --- a/spec/features/explore/new_menu_spec.rb +++ b/spec/features/explore/new_menu_spec.rb @@ -29,7 +29,7 @@ describe 'Top Plus Menu', :js do click_topmenuitem("New group") - expect(page).to have_content('Group path') + expect(page).to have_content('Group URL') expect(page).to have_content('Group name') end @@ -79,7 +79,7 @@ describe 'Top Plus Menu', :js do click_topmenuitem("New subgroup") - expect(page).to have_content('Group path') + expect(page).to have_content('Group URL') expect(page).to have_content('Group name') end diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb index 63aa26cf5fd..4d04b8043ec 100644 --- a/spec/features/groups_spec.rb +++ b/spec/features/groups_spec.rb @@ -7,7 +7,7 @@ describe 'Group' do matcher :have_namespace_error_message do match do |page| - page.has_content?("Path can contain only letters, digits, '_', '-' and '.'. Cannot start with '-' or end in '.', '.git' or '.atom'.") + page.has_content?("Group URL can contain only letters, digits, '_', '-' and '.'. Cannot start with '-' or end in '.', '.git' or '.atom'.") end end @@ -18,7 +18,7 @@ describe 'Group' do describe 'with space in group path' do it 'renders new group form with validation errors' do - fill_in 'Group path', with: 'space group' + fill_in 'Group URL', with: 'space group' click_button 'Create group' expect(current_path).to eq(groups_path) @@ -28,7 +28,7 @@ describe 'Group' do describe 'with .atom at end of group path' do it 'renders new group form with validation errors' do - fill_in 'Group path', with: 'atom_group.atom' + fill_in 'Group URL', with: 'atom_group.atom' click_button 'Create group' expect(current_path).to eq(groups_path) @@ -38,7 +38,7 @@ describe 'Group' do describe 'with .git at end of group path' do it 'renders new group form with validation errors' do - fill_in 'Group path', with: 'git_group.git' + fill_in 'Group URL', with: 'git_group.git' click_button 'Create group' expect(current_path).to eq(groups_path) @@ -94,7 +94,8 @@ describe 'Group' do end it 'creates a nested group' do - fill_in 'Group path', with: 'bar' + fill_in 'Group name', with: 'bar' + fill_in 'Group URL', with: 'bar' click_button 'Create group' expect(current_path).to eq(group_path('foo/bar')) @@ -112,7 +113,8 @@ describe 'Group' do visit new_group_path(group, parent_id: group.id) - fill_in 'Group path', with: 'bar' + fill_in 'Group name', with: 'bar' + fill_in 'Group URL', with: 'bar' click_button 'Create group' expect(current_path).to eq(group_path('foo/bar')) diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index d71ccfb4334..1289d3ce01f 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/spec/services/groups/transfer_service_spec.rb @@ -177,7 +177,7 @@ describe Groups::TransferService, :postgresql do it 'should add an error on group' do transfer_service.execute(new_parent_group) - expect(transfer_service.error).to eq('Transfer failed: Validation failed: Path has already been taken') + expect(transfer_service.error).to eq('Transfer failed: Validation failed: Group URL has already been taken') end end -- cgit v1.2.1