Merge branch '254931-better-ux-for-group-creation' into 'master'

Better UX for Group Creation

See merge request gitlab-org/gitlab!61100

app/assets/javascripts/pages/groups/new/components/app.vue

+<script>
+import importGroupIllustration from '@gitlab/svgs/dist/illustrations/group-import.svg';
+import newGroupIllustration from '@gitlab/svgs/dist/illustrations/group-new.svg';
+
+import { s__ } from '~/locale';
+import NewNamespacePage from '~/vue_shared/new_namespace/new_namespace_page.vue';
+import createGroupDescriptionDetails from './create_group_description_details.vue';
+
+const PANELS = [
+  {
+    name: 'create-group-pane',
+    selector: '#create-group-pane',
+    title: s__('GroupsNew|Create group'),
+    description: s__(
+      'GroupsNew|Assemble related projects together and grant members access to several projects at once.',
+    ),
+    illustration: newGroupIllustration,
+    details: createGroupDescriptionDetails,
+  },
+  {
+    name: 'import-group-pane',
+    selector: '#import-group-pane',
+    title: s__('GroupsNew|Import group'),
+    description: s__(
+      'GroupsNew|Export groups with all their related data and move to a new GitLab instance.',
+    ),
+    illustration: importGroupIllustration,
+    details: 'Migrate your existing groups from another instance of GitLab.',
+  },
+];
+
+export default {
+  components: {
+    NewNamespacePage,
+  },
+  props: {
+    hasErrors: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  PANELS,
+};
+</script>
+
+<template>
+  <new-namespace-page
+    :jump-to-last-persisted-panel="hasErrors"
+    :initial-breadcrumb="s__('New group')"
+    :panels="$options.PANELS"
+    :title="s__('GroupsNew|Create new group')"
+    persistence-key="new_group_last_active_tab"
+  />
+</template>

app/assets/javascripts/pages/groups/new/components/create_group_description_details.vue

+<script>
+import { GlSprintf, GlLink } from '@gitlab/ui';
+import { helpPagePath } from '~/helpers/help_page_helper';
+
+export default {
+  components: {
+    GlLink,
+    GlSprintf,
+  },
+  paths: {
+    groupsHelpPath: helpPagePath('user/group/index'),
+    subgroupsHelpPath: helpPagePath('user/group/subgroups/index'),
+  },
+};
+</script>
+
+<template>
+  <div>
+    <p>
+      <gl-sprintf
+        :message="
+          s__(
+            'GroupsNew|%{linkStart}Groups%{linkEnd} allow you to manage and collaborate across multiple projects. Members of a group have access to all of its projects. Groups can also be nested by creating subgroups.',
+          )
+        "
+      >
+        <template #link="{ content }">
+          <gl-link :href="$options.paths.groupsHelpPath" target="_blank">{{ content }}</gl-link>
+        </template>
+      </gl-sprintf>
+    </p>
+    <p>
+      <gl-sprintf
+        :message="
+          s__('GroupsNew|Groups can also be nested by creating %{linkStart}subgroups%{linkEnd}.')
+        "
+      >
+        <template #link="{ content }">
+          <gl-link :href="$options.paths.subgroupsHelpPath" target="_blank">{{ content }}</gl-link>
+        </template>
+      </gl-sprintf>
+    </p>
+  </div>
+</template>

app/assets/javascripts/pages/groups/new/index.js

-import $ from 'jquery';
+import Vue from 'vue';
 import BindInOut from '~/behaviors/bind_in_out';
 import initFilePickers from '~/file_pickers';
 import Group from '~/group';
-import LinkedTabs from '~/lib/utils/bootstrap_linked_tabs';
+import { parseBoolean } from '~/lib/utils/common_utils';
+import NewGroupCreationApp from './components/app.vue';
 import GroupPathValidator from './group_path_validator';
 
 new GroupPathValidator(); // eslint-disable-line no-new
@@ -12,15 +13,21 @@ initFilePickers();
 
 new Group(); // eslint-disable-line no-new
 
-const CONTAINER_SELECTOR = '.group-edit-container .nav-tabs';
-const DEFAULT_ACTION = '#create-group-pane';
-// eslint-disable-next-line no-new
-new LinkedTabs({
-  defaultAction: DEFAULT_ACTION,
-  parentEl: CONTAINER_SELECTOR,
-  hashedTabs: true,
-});
-
-if (window.location.hash) {
-  $(CONTAINER_SELECTOR).find(`a[href="${window.location.hash}"]`).tab('show');
+function initNewGroupCreation(el) {
+  const { hasErrors } = el.dataset;
+
+  const props = {
+    hasErrors: parseBoolean(hasErrors),
+  };
+
+  return new Vue({
+    el,
+    render(h) {
+      return h(NewGroupCreationApp, { props });
+    },
+  });
 }
+
+const el = document.querySelector('.js-new-group-creation');
+
+initNewGroupCreation(el);

app/assets/javascripts/vue_shared/new_namespace/new_namespace_page.vue

@@ -129,7 +129,7 @@ export default {
           <gl-icon name="chevron-right" :size="8" />
         </template>
       </gl-breadcrumb>
-      <legacy-container :key="activePanel.name" class="gl-mt-3" :selector="activePanel.selector" />
+      <legacy-container :key="activePanel.name" :selector="activePanel.selector" />
     </div>
   </div>
 </template>

app/views/groups/_new_group_fields.html.haml

@@ -22,6 +22,6 @@
     .col-sm-4
       = recaptcha_tags
 .row
-  .form-actions.col-sm-12
+  .col-sm-12
     = f.submit _('Create group'), class: "btn gl-button btn-confirm"
     = link_to _('Cancel'), dashboard_groups_path, class: 'btn gl-button btn-default btn-cancel'

app/views/groups/new.html.haml

@@ -2,47 +2,24 @@
 - @hide_top_links = true
 - page_title    _('New Group')
 - header_title  _("Groups"), dashboard_groups_path
-- active_tab = local_assigns.fetch(:active_tab, 'create')
+- add_page_specific_style 'page_bundles/new_namespace'
 
-.group-edit-container.gl-mt-3
-  .row
-    .col-lg-3.group-settings-sidebar
-      %h4.prepend-top-0
-        = _('New group')
-      %p
-        - group_docs_path = help_page_path('user/group/index')
-        - group_docs_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: group_docs_path }
-        = s_('%{group_docs_link_start}Groups%{group_docs_link_end} allow you to manage and collaborate across multiple projects. Members of a group have access to all of its projects.').html_safe % { group_docs_link_start: group_docs_link_start, group_docs_link_end: '</a>'.html_safe }
-      %p
-        - subgroup_docs_path = help_page_path('user/group/subgroups/index')
-        - subgroup_docs_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.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: '</a>'.html_safe }
-      %p
-        = _('Projects that belong to a group are prefixed with the group namespace. Existing projects may be moved into a group.')
+.group-edit-container.gl-mt-5
 
-    .col-lg-9.js-toggle-container
-      %ul.nav.nav-tabs.nav-links.gitlab-tabs{ role: 'tablist' }
-        %li.nav-item{ role: 'presentation' }
-          %a.nav-link.active{ href: '#create-group-pane', id: 'create-group-tab', role: 'tab', data: { toggle: 'tab', track_label: 'create_group', track_event: 'click_tab', track_value: '' } }
-            %span.d-none.d-sm-block= s_('GroupsNew|Create group')
-            %span.d-block.d-sm-none= s_('GroupsNew|Create')
-        %li.nav-item{ role: 'presentation' }
-          %a.nav-link{ href: '#import-group-pane', id: 'import-group-tab', role: 'tab', data: { toggle: 'tab', track_label: 'import_group', track_event: 'click_tab', track_value: '' } }
-            %span.d-none.d-sm-block= s_('GroupsNew|Import group')
-            %span.d-block.d-sm-none= s_('GroupsNew|Import')
+  .js-new-group-creation{ data: { has_errors: @group.errors.any?.to_s } }
 
-      .tab-content.gitlab-tab-content.gl-border-none
-        .tab-pane.js-toggle-container{ id: 'create-group-pane', class: active_when(active_tab == 'create'), role: 'tabpanel' }
-          = form_for @group, html: { class: 'group-form gl-show-field-errors' } do |f|
-            = render 'new_group_fields', f: f, group_name_id: 'create-group-name'
+  .row{ 'v-cloak': true }
+    #create-group-pane.tab-pane
+      = form_for @group, html: { class: 'group-form gl-show-field-errors' } do |f|
+        = render 'new_group_fields', f: f, group_name_id: 'create-group-name'
 
-        .tab-pane.no-padding.js-toggle-container{ id: 'import-group-pane', class: active_when(active_tab) == 'import', role: 'tabpanel' }
-          - if import_sources_enabled?
-            - if Feature.enabled?(:bulk_import)
-              = render 'import_group_from_another_instance_panel'
-              .gl-mt-7.gl-border-b-solid.gl-border-gray-100.gl-border-1
-            = render 'import_group_from_file_panel'
-          - else
-            .nothing-here-block
-              %h4= s_('GroupsNew|No import options available')
-              %p= s_('GroupsNew|Contact an administrator to enable options for importing your group.')
+    #import-group-pane.tab-pane
+      - if import_sources_enabled?
+        - if Feature.enabled?(:bulk_import)
+          = render 'import_group_from_another_instance_panel'
+          .gl-mt-7.gl-border-b-solid.gl-border-gray-100.gl-border-1
+        = render 'import_group_from_file_panel'
+      - else
+        .nothing-here-block
+          %h4= s_('GroupsNew|No import options available')
+          %p= s_('GroupsNew|Contact an administrator to enable options for importing your group.')

doc/user/group/import/index.md

@@ -105,7 +105,7 @@ on an existing group's page.
 
    ![Navigation paths to create a new group](img/new_group_navigation_v13_8.png)
 
-1. On the New Group page, select the **Import group** tab.
+1. On the New Group page, select **Import group**.
 
    ![Fill in import details](img/import_panel_v13_8.png)
 

doc/user/group/index.md

@@ -51,6 +51,7 @@ To create a group:
 1. From the top menu, either:
    - Select **Groups > Your Groups**, and on the right, select the **New group** button.
    - To the left of the search box, select the plus sign and then **New group**.
+1. Select **Create group**.
 1. For the **Group name**, use only:
    - Alphanumeric characters
    - Emojis

locale/gitlab.pot

@@ -558,9 +558,6 @@ msgstr ""
 msgid "%{global_id} is not a valid ID for %{expected_types}."
 msgstr ""
 
-msgid "%{group_docs_link_start}Groups%{group_docs_link_end} allow you to manage and collaborate across multiple projects. Members of a group have access to all of its projects."
-msgstr ""
-
 msgid "%{group_name} activity"
 msgstr ""
 
@@ -16085,9 +16082,6 @@ msgstr ""
 msgid "Groups and subgroups"
 msgstr ""
 
-msgid "Groups can also be nested by creating %{subgroup_docs_link_start}subgroups%{subgroup_docs_link_end}."
-msgstr ""
-
 msgid "Groups to synchronize"
 msgstr ""
 
@@ -16124,22 +16118,31 @@ msgstr ""
 msgid "GroupsEmptyState|You can manage your group member’s permissions and access to each project in the group."
 msgstr ""
 
+msgid "GroupsNew|%{linkStart}Groups%{linkEnd} allow you to manage and collaborate across multiple projects. Members of a group have access to all of its projects. Groups can also be nested by creating subgroups."
+msgstr ""
+
+msgid "GroupsNew|Assemble related projects together and grant members access to several projects at once."
+msgstr ""
+
 msgid "GroupsNew|Connect instance"
 msgstr ""
 
 msgid "GroupsNew|Contact an administrator to enable options for importing your group."
 msgstr ""
 
-msgid "GroupsNew|Create"
+msgid "GroupsNew|Create group"
 msgstr ""
 
-msgid "GroupsNew|Create group"
+msgid "GroupsNew|Create new group"
+msgstr ""
+
+msgid "GroupsNew|Export groups with all their related data and move to a new GitLab instance."
 msgstr ""
 
 msgid "GroupsNew|GitLab source URL"
 msgstr ""
 
-msgid "GroupsNew|Import"
+msgid "GroupsNew|Groups can also be nested by creating %{linkStart}subgroups%{linkEnd}."
 msgstr ""
 
 msgid "GroupsNew|Import group"
@@ -26160,9 +26163,6 @@ 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 "Projects to index"
 msgstr ""
 

qa/qa/page/group/new.rb

@@ -38,6 +38,14 @@ module QA
           fill_element(:import_gitlab_token, token)
         end
 
+        def click_import_group
+          click_on 'Import group'
+        end
+
+        def click_create_group
+          click_on 'Create group'
+        end
+
         # Connect gitlab instance
         #
         # @param [String] gitlab_url

qa/qa/resource/group.rb

@@ -29,6 +29,7 @@ module QA
             group_show.go_to_new_subgroup
 
             Page::Group::New.perform do |group_new|
+              group_new.click_create_group
               group_new.set_path(path)
               group_new.set_visibility('Public')
               group_new.create

qa/qa/specs/features/browser_ui/1_manage/group/create_group_with_mattermost_team_spec.rb

@@ -10,9 +10,13 @@ module QA
         Page::Dashboard::Groups.perform do |groups|
           groups.click_new_group
 
-          expect(groups).to have_content(
-            /Create a Mattermost team for this group/
-          )
+          Page::Group::New.perform do |group_new|
+            group_new.click_create_group
+
+            expect(group_new).to have_content(
+              /Create a Mattermost team for this group/
+            )
+          end
         end
       end
     end

spec/features/dashboard/group_spec.rb

@@ -16,6 +16,8 @@ RSpec.describe 'Dashboard Group' do
   it 'creates new group', :js do
     visit dashboard_groups_path
     find('[data-testid="new-group-button"]').click
+    click_link 'Create group'
+
     new_name = 'Samurai'
 
     fill_in 'group_name', with: new_name

spec/features/groups/import_export/connect_instance_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe 'Import/Export - Connect to another instance', :js do
 
     visit new_group_path
 
-    find('#import-group-tab').click
+    click_link 'Import group'
   end
 
   context 'when the user provides valid credentials' do

spec/features/groups/import_export/import_file_spec.rb

@@ -28,9 +28,9 @@ RSpec.describe 'Import/Export - Group Import', :js do
         group_name = 'Test Group Import'
 
         visit new_group_path
+        click_link 'Import group'
 
-        fill_in :group_name, with: group_name
-        find('#import-group-tab').click
+        fill_in :import_group_name, with: group_name
 
         expect(page).to have_content 'Import group from file'
         attach_file(file) do
@@ -51,9 +51,9 @@ RSpec.describe 'Import/Export - Group Import', :js do
     context 'when modifying the pre-filled path' do
       it 'successfully imports the group' do
         visit new_group_path
+        click_link 'Import group'
 
-        fill_in :group_name, with: 'Test Group Import'
-        find('#import-group-tab').click
+        fill_in :import_group_name, with: 'Test Group Import'
 
         fill_in :import_group_path, with: 'custom-path'
         attach_file(file) do
@@ -74,7 +74,7 @@ RSpec.describe 'Import/Export - Group Import', :js do
 
       it 'suggests a unique path' do
         visit new_group_path
-        find('#import-group-tab').click
+        click_link 'Import group'
 
         fill_in :import_group_path, with: 'test-group-import'
         expect(page).to have_content 'Group path is already taken. Suggestions: test-group-import1'
@@ -87,9 +87,9 @@ RSpec.describe 'Import/Export - Group Import', :js do
 
     it 'displays an error' do
       visit new_group_path
+      click_link 'Import group'
 
-      fill_in :group_name, with: 'Test Group Import'
-      find('#import-group-tab').click
+      fill_in :import_group_name, with: 'Test Group Import'
       attach_file(file) do
         find('.js-filepicker-button').click
       end

spec/features/groups_spec.rb

@@ -15,9 +15,10 @@ RSpec.describe 'Group' do
     end
   end
 
-  describe 'create a group' do
+  describe 'create a group', :js do
     before do
       visit new_group_path
+      click_link 'Create group'
     end
 
     describe 'as a non-admin' do
@@ -50,13 +51,14 @@ RSpec.describe 'Group' do
         fill_in 'Group URL', with: 'space group'
         click_button 'Create group'
 
-        expect(current_path).to eq(groups_path)
-        expect(page).to have_namespace_error_message
+        expect(current_path).to eq(new_group_path)
+        expect(page).to have_text('Please choose a group URL with no special characters.')
       end
     end
 
     describe 'with .atom at end of group path' do
       it 'renders new group form with validation errors' do
+        fill_in 'Group name', with: 'test-group'
         fill_in 'Group URL', with: 'atom_group.atom'
         click_button 'Create group'
 
@@ -67,6 +69,7 @@ RSpec.describe 'Group' do
 
     describe 'with .git at end of group path' do
       it 'renders new group form with validation errors' do
+        fill_in 'Group name', with: 'test-group'
         fill_in 'Group URL', with: 'git_group.git'
         click_button 'Create group'
 
@@ -109,6 +112,7 @@ RSpec.describe 'Group' do
         stub_mattermost_setting(enabled: mattermost_enabled)
 
         visit new_group_path
+        click_link 'Create group'
       end
 
       context 'Mattermost enabled' do
@@ -119,7 +123,7 @@ RSpec.describe 'Group' do
         end
 
         it 'unchecks the checkbox by default' do
-          expect(find('#group_create_chat_team')['checked']).to eq(false)
+          expect(find('#group_create_chat_team')).not_to be_checked
         end
 
         it 'updates the team URL on graph path update', :js do
@@ -147,6 +151,7 @@ RSpec.describe 'Group' do
         stub_application_setting(recaptcha_enabled: true)
         allow(Gitlab::Recaptcha).to receive(:load_configurations!)
         visit new_group_path
+        click_link 'Create group'
       end
 
       it 'renders recaptcha' do
@@ -159,6 +164,7 @@ RSpec.describe 'Group' do
         stub_feature_flags(recaptcha_on_top_level_group_creation: false)
         stub_application_setting(recaptcha_enabled: true)
         visit new_group_path
+        click_link 'Create group'
       end
 
       it 'does not render recaptcha' do
@@ -167,30 +173,30 @@ RSpec.describe 'Group' do
     end
   end
 
-  describe 'create a nested group' do
+  describe 'create a nested group', :js do
     let_it_be(:group) { create(:group, path: 'foo') }
 
     context 'as admin' do
       let(:user) { create(:admin) }
 
       before do
-        visit new_group_path(group, parent_id: group.id)
+        visit new_group_path(parent_id: group.id)
       end
 
       context 'when admin mode is enabled', :enable_admin_mode do
         it 'creates a nested group' do
+          click_link 'Create group'
           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'))
-          expect(page).to have_content("Group 'bar' was successfully created.")
+          expect(page).to have_selector 'h1', text: 'bar'
         end
       end
 
       context 'when admin mode is disabled' do
         it 'is not allowed' do
-          expect(page).to have_gitlab_http_status(:not_found)
+          expect(page).not_to have_button('Create group')
         end
       end
     end
@@ -203,14 +209,14 @@ RSpec.describe 'Group' do
         sign_out(:user)
         sign_in(user)
 
-        visit new_group_path(group, parent_id: group.id)
+        visit new_group_path(parent_id: group.id)
+        click_link 'Create group'
 
         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'))
-        expect(page).to have_content("Group 'bar' was successfully created.")
+        expect(page).to have_selector 'h1', text: 'bar'
       end
     end
 
@@ -221,7 +227,7 @@ RSpec.describe 'Group' do
       end
 
       context 'when creating subgroup' do
-        let(:path) { new_group_path(group, parent_id: group.id) }
+        let(:path) { new_group_path(parent_id: group.id) }
 
         it 'does not render recaptcha' do
           visit path
@@ -237,6 +243,7 @@ RSpec.describe 'Group' do
       before do
         group.add_owner(user)
         visit new_group_path(parent_id: group.id)
+        click_link 'Create group'
       end
 
       it 'shows a message if group url is available' do
@@ -255,14 +262,15 @@ RSpec.describe 'Group' do
     end
   end
 
-  it 'checks permissions to avoid exposing groups by parent_id' do
+  it 'checks permissions to avoid exposing groups by parent_id', :js do
     group = create(:group, :private, path: 'secret-group')
 
     sign_out(:user)
     sign_in(create(:user))
     visit new_group_path(parent_id: group.id)
 
-    expect(page).not_to have_content('secret-group')
+    expect(page).to have_title('Not Found')
+    expect(page).to have_content('Page Not Found')
   end
 
   describe 'group edit', :js do