From 2ade712988934c1dc96191e213896e5211053531 Mon Sep 17 00:00:00 2001
From: Tom Quirk <tquirk@gitlab.com>
Date: Wed, 4 Aug 2021 15:53:58 +0000
Subject: [PATCH] UI polish for Jira Connect Create branch
- Replace success alert with empty state
- Dynamic page title
---
.../branches/components/new_branch_form.vue | 105 +++++++-----------
.../jira_connect/branches/constants.js | 15 ++-
.../jira_connect/branches/index.js | 14 +--
.../jira_connect/branches/pages/index.vue | 60 ++++++++++
.../jira_connect/branches_controller.rb | 21 +++-
app/views/jira_connect/branches/new.html.haml | 2 +-
locale/gitlab.pot | 3 +
.../jira_connect/branches_controller_spec.rb | 9 +-
.../components/new_branch_form_spec.js | 26 +++--
.../jira_connect/branches/pages/index_spec.js | 65 +++++++++++
10 files changed, 223 insertions(+), 97 deletions(-)
create mode 100644 app/assets/javascripts/jira_connect/branches/pages/index.vue
create mode 100644 spec/frontend/jira_connect/branches/pages/index_spec.js
diff --git a/app/assets/javascripts/jira_connect/branches/components/new_branch_form.vue b/app/assets/javascripts/jira_connect/branches/components/new_branch_form.vue
index b2cc3a315cca9..66fcb8e10eb01 100644
--- a/app/assets/javascripts/jira_connect/branches/components/new_branch_form.vue
+++ b/app/assets/javascripts/jira_connect/branches/components/new_branch_form.vue
@@ -3,8 +3,6 @@ import { GlFormGroup, GlButton, GlFormInput, GlForm, GlAlert } from '@gitlab/ui'
import {
CREATE_BRANCH_ERROR_GENERIC,
CREATE_BRANCH_ERROR_WITH_CONTEXT,
- CREATE_BRANCH_SUCCESS_ALERT,
- I18N_NEW_BRANCH_PAGE_TITLE,
I18N_NEW_BRANCH_LABEL_DROPDOWN,
I18N_NEW_BRANCH_LABEL_BRANCH,
I18N_NEW_BRANCH_LABEL_SOURCE,
@@ -19,8 +17,6 @@ const DEFAULT_ALERT_PARAMS = {
title: '',
message: '',
variant: DEFAULT_ALERT_VARIANT,
- primaryButtonLink: '',
- primaryButtonText: '',
};
export default {
@@ -34,13 +30,7 @@ export default {
ProjectDropdown,
SourceBranchDropdown,
},
- props: {
- initialBranchName: {
- type: String,
- required: false,
- default: '',
- },
- },
+ inject: ['initialBranchName'],
data() {
return {
selectedProject: null,
@@ -111,10 +101,7 @@ export default {
message: errors[0],
});
} else {
- this.displayAlert({
- ...CREATE_BRANCH_SUCCESS_ALERT,
- variant: 'success',
- });
+ this.$emit('success');
}
} catch (e) {
this.onError({
@@ -126,7 +113,6 @@ export default {
},
},
i18n: {
- I18N_NEW_BRANCH_PAGE_TITLE,
I18N_NEW_BRANCH_LABEL_DROPDOWN,
I18N_NEW_BRANCH_LABEL_BRANCH,
I18N_NEW_BRANCH_LABEL_SOURCE,
@@ -134,15 +120,8 @@ export default {
},
};
</script>
-
<template>
- <div>
- <div class="gl-border-1 gl-border-b-solid gl-border-gray-100 gl-mb-5 gl-mt-7">
- <h1 class="page-title">
- {{ $options.i18n.I18N_NEW_BRANCH_PAGE_TITLE }}
- </h1>
- </div>
-
+ <gl-form @submit.prevent="onSubmit">
<gl-alert
v-if="showAlert"
class="gl-mb-5"
@@ -152,50 +131,44 @@ export default {
>
{{ alertParams.message }}
</gl-alert>
+ <gl-form-group :label="$options.i18n.I18N_NEW_BRANCH_LABEL_DROPDOWN" label-for="project-select">
+ <project-dropdown
+ id="project-select"
+ :selected-project="selectedProject"
+ @change="onProjectSelect"
+ @error="onError"
+ />
+ </gl-form-group>
- <gl-form @submit.prevent="onSubmit">
- <gl-form-group
- :label="$options.i18n.I18N_NEW_BRANCH_LABEL_DROPDOWN"
- label-for="project-select"
- >
- <project-dropdown
- id="project-select"
- :selected-project="selectedProject"
- @change="onProjectSelect"
- @error="onError"
- />
- </gl-form-group>
+ <gl-form-group
+ :label="$options.i18n.I18N_NEW_BRANCH_LABEL_BRANCH"
+ label-for="branch-name-input"
+ >
+ <gl-form-input id="branch-name-input" v-model="branchName" type="text" required />
+ </gl-form-group>
- <gl-form-group
- :label="$options.i18n.I18N_NEW_BRANCH_LABEL_BRANCH"
- label-for="branch-name-input"
- >
- <gl-form-input id="branch-name-input" v-model="branchName" type="text" required />
- </gl-form-group>
+ <gl-form-group
+ :label="$options.i18n.I18N_NEW_BRANCH_LABEL_SOURCE"
+ label-for="source-branch-select"
+ >
+ <source-branch-dropdown
+ id="source-branch-select"
+ :selected-project="selectedProject"
+ :selected-branch-name="selectedSourceBranchName"
+ @change="onSourceBranchSelect"
+ @error="onError"
+ />
+ </gl-form-group>
- <gl-form-group
- :label="$options.i18n.I18N_NEW_BRANCH_LABEL_SOURCE"
- label-for="source-branch-select"
+ <div class="form-actions">
+ <gl-button
+ :loading="createBranchLoading"
+ type="submit"
+ variant="confirm"
+ :disabled="disableSubmitButton"
>
- <source-branch-dropdown
- id="source-branch-select"
- :selected-project="selectedProject"
- :selected-branch-name="selectedSourceBranchName"
- @change="onSourceBranchSelect"
- @error="onError"
- />
- </gl-form-group>
-
- <div class="form-actions">
- <gl-button
- :loading="createBranchLoading"
- type="submit"
- variant="confirm"
- :disabled="disableSubmitButton"
- >
- {{ $options.i18n.I18N_NEW_BRANCH_SUBMIT_BUTTON_TEXT }}
- </gl-button>
- </div>
- </gl-form>
- </div>
+ {{ $options.i18n.I18N_NEW_BRANCH_SUBMIT_BUTTON_TEXT }}
+ </gl-button>
+ </div>
+ </gl-form>
</template>
diff --git a/app/assets/javascripts/jira_connect/branches/constants.js b/app/assets/javascripts/jira_connect/branches/constants.js
index 7095f123a9e22..ab9d3b2c110f0 100644
--- a/app/assets/javascripts/jira_connect/branches/constants.js
+++ b/app/assets/javascripts/jira_connect/branches/constants.js
@@ -3,7 +3,6 @@ import { __, s__ } from '~/locale';
export const BRANCHES_PER_PAGE = 20;
export const PROJECTS_PER_PAGE = 20;
-export const I18N_NEW_BRANCH_PAGE_TITLE = __('New branch');
export const I18N_NEW_BRANCH_LABEL_DROPDOWN = __('Project');
export const I18N_NEW_BRANCH_LABEL_BRANCH = __('Branch name');
export const I18N_NEW_BRANCH_LABEL_SOURCE = __('Source branch');
@@ -14,7 +13,13 @@ export const CREATE_BRANCH_ERROR_GENERIC = s__(
);
export const CREATE_BRANCH_ERROR_WITH_CONTEXT = s__('JiraConnect|Failed to create branch.');
-export const CREATE_BRANCH_SUCCESS_ALERT = {
- title: s__('JiraConnect|New branch was successfully created.'),
- message: s__('JiraConnect|You can now close this window and return to Jira.'),
-};
+export const I18N_PAGE_TITLE_WITH_BRANCH_NAME = s__(
+ 'JiraConnect|Create branch for Jira issue %{jiraIssue}',
+);
+export const I18N_PAGE_TITLE_DEFAULT = __('New branch');
+export const I18N_NEW_BRANCH_SUCCESS_TITLE = s__(
+ 'JiraConnect|New branch was successfully created.',
+);
+export const I18N_NEW_BRANCH_SUCCESS_MESSAGE = s__(
+ 'JiraConnect|You can now close this window and return to Jira.',
+);
diff --git a/app/assets/javascripts/jira_connect/branches/index.js b/app/assets/javascripts/jira_connect/branches/index.js
index b8fe255e31061..95bd4f5c6752f 100644
--- a/app/assets/javascripts/jira_connect/branches/index.js
+++ b/app/assets/javascripts/jira_connect/branches/index.js
@@ -1,6 +1,6 @@
import Vue from 'vue';
import VueApollo from 'vue-apollo';
-import JiraConnectNewBranchForm from '~/jira_connect/branches/components/new_branch_form.vue';
+import JiraConnectNewBranchPage from '~/jira_connect/branches/pages/index.vue';
import createDefaultClient from '~/lib/graphql';
Vue.use(VueApollo);
@@ -11,7 +11,7 @@ export default async function initJiraConnectBranches() {
return null;
}
- const { initialBranchName } = el.dataset;
+ const { initialBranchName, successStateSvgPath } = el.dataset;
const apolloProvider = new VueApollo({
defaultClient: createDefaultClient(
@@ -25,12 +25,12 @@ export default async function initJiraConnectBranches() {
return new Vue({
el,
apolloProvider,
+ provide: {
+ initialBranchName,
+ successStateSvgPath,
+ },
render(createElement) {
- return createElement(JiraConnectNewBranchForm, {
- props: {
- initialBranchName,
- },
- });
+ return createElement(JiraConnectNewBranchPage);
},
});
}
diff --git a/app/assets/javascripts/jira_connect/branches/pages/index.vue b/app/assets/javascripts/jira_connect/branches/pages/index.vue
new file mode 100644
index 0000000000000..d72dec6cdee9f
--- /dev/null
+++ b/app/assets/javascripts/jira_connect/branches/pages/index.vue
@@ -0,0 +1,60 @@
+<script>
+import { GlEmptyState } from '@gitlab/ui';
+import { sprintf } from '~/locale';
+import NewBranchForm from '../components/new_branch_form.vue';
+import {
+ I18N_PAGE_TITLE_WITH_BRANCH_NAME,
+ I18N_PAGE_TITLE_DEFAULT,
+ I18N_NEW_BRANCH_SUCCESS_TITLE,
+ I18N_NEW_BRANCH_SUCCESS_MESSAGE,
+} from '../constants';
+
+export default {
+ components: {
+ GlEmptyState,
+ NewBranchForm,
+ },
+ inject: ['initialBranchName', 'successStateSvgPath'],
+ data() {
+ return {
+ showForm: true,
+ };
+ },
+ computed: {
+ pageTitle() {
+ return this.initialBranchName
+ ? sprintf(this.$options.i18n.I18N_PAGE_TITLE_WITH_BRANCH_NAME, {
+ jiraIssue: this.initialBranchName,
+ })
+ : this.$options.i18n.I18N_PAGE_TITLE_DEFAULT;
+ },
+ },
+ methods: {
+ onNewBranchFormSuccess() {
+ // light-weight toggle to hide the form and show the success state
+ this.showForm = false;
+ },
+ },
+ i18n: {
+ I18N_PAGE_TITLE_WITH_BRANCH_NAME,
+ I18N_PAGE_TITLE_DEFAULT,
+ I18N_NEW_BRANCH_SUCCESS_TITLE,
+ I18N_NEW_BRANCH_SUCCESS_MESSAGE,
+ },
+};
+</script>
+<template>
+ <div>
+ <div class="gl-border-1 gl-border-b-solid gl-border-gray-100 gl-mb-5 gl-mt-7">
+ <h1 data-testid="page-title" class="page-title">{{ pageTitle }}</h1>
+ </div>
+
+ <new-branch-form v-if="showForm" @success="onNewBranchFormSuccess" />
+ <gl-empty-state
+ v-else
+ :title="$options.i18n.I18N_NEW_BRANCH_SUCCESS_TITLE"
+ :description="$options.i18n.I18N_NEW_BRANCH_SUCCESS_MESSAGE"
+ :svg-path="successStateSvgPath"
+ />
+ </div>
+</template>
diff --git a/app/controllers/jira_connect/branches_controller.rb b/app/controllers/jira_connect/branches_controller.rb
index 7d7faae62a5f7..97d0b75639e93 100644
--- a/app/controllers/jira_connect/branches_controller.rb
+++ b/app/controllers/jira_connect/branches_controller.rb
@@ -8,20 +8,31 @@ class JiraConnect::BranchesController < ApplicationController
feature_category :integrations
def new
+ @new_branch_data = new_branch_data
+ end
+
+ def self.feature_enabled?(user)
+ Feature.enabled?(:jira_connect_create_branch, user, default_enabled: :yaml)
+ end
+
+ private
+
+ def initial_branch_name
return unless params[:issue_key].present?
- @branch_name = Issue.to_branch_name(
+ Issue.to_branch_name(
params[:issue_key],
params[:issue_summary]
)
end
- def self.feature_enabled?(user)
- Feature.enabled?(:jira_connect_create_branch, user, default_enabled: :yaml)
+ def new_branch_data
+ {
+ initial_branch_name: initial_branch_name,
+ success_state_svg_path: ActionController::Base.helpers.image_path('illustrations/merge_requests.svg')
+ }
end
- private
-
def feature_enabled!
render_404 unless self.class.feature_enabled?(current_user)
end
diff --git a/app/views/jira_connect/branches/new.html.haml b/app/views/jira_connect/branches/new.html.haml
index ec2b7be47ca6f..f0e34c3001869 100644
--- a/app/views/jira_connect/branches/new.html.haml
+++ b/app/views/jira_connect/branches/new.html.haml
@@ -2,4 +2,4 @@
- @hide_top_links = true
- page_title _('New branch')
-.js-jira-connect-create-branch{ data: { initial_branch_name: @branch_name } }
+.js-jira-connect-create-branch{ data: @new_branch_data }
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 22e6e93d5535c..cd5002e351f08 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -18692,6 +18692,9 @@ msgstr ""
msgid "Jira-GitLab user mapping template"
msgstr ""
+msgid "JiraConnect|Create branch for Jira issue %{jiraIssue}"
+msgstr ""
+
msgid "JiraConnect|Failed to create branch."
msgstr ""
diff --git a/spec/controllers/jira_connect/branches_controller_spec.rb b/spec/controllers/jira_connect/branches_controller_spec.rb
index 4b198eb882047..31f6860891829 100644
--- a/spec/controllers/jira_connect/branches_controller_spec.rb
+++ b/spec/controllers/jira_connect/branches_controller_spec.rb
@@ -15,21 +15,24 @@
get :new, params: { issue_key: 'ACME-123', issue_summary: 'My Issue !@#$%' }
expect(response).to be_successful
- expect(assigns(:branch_name)).to eq('ACME-123-my-issue')
+ expect(assigns(:new_branch_data)).to include(
+ initial_branch_name: 'ACME-123-my-issue',
+ success_state_svg_path: start_with('/assets/illustrations/merge_requests-')
+ )
end
it 'ignores missing summary' do
get :new, params: { issue_key: 'ACME-123' }
expect(response).to be_successful
- expect(assigns(:branch_name)).to eq('ACME-123')
+ expect(assigns(:new_branch_data)).to include(initial_branch_name: 'ACME-123')
end
it 'does not set a branch name if key is not passed' do
get :new, params: { issue_summary: 'My issue' }
expect(response).to be_successful
- expect(assigns(:branch_name)).to be_nil
+ expect(assigns(:new_branch_data)).to include('initial_branch_name': nil)
end
context 'when feature flag is disabled' do
diff --git a/spec/frontend/jira_connect/branches/components/new_branch_form_spec.js b/spec/frontend/jira_connect/branches/components/new_branch_form_spec.js
index 7ea47c2737ab3..7326b84ad54ec 100644
--- a/spec/frontend/jira_connect/branches/components/new_branch_form_spec.js
+++ b/spec/frontend/jira_connect/branches/components/new_branch_form_spec.js
@@ -9,7 +9,6 @@ import SourceBranchDropdown from '~/jira_connect/branches/components/source_bran
import {
CREATE_BRANCH_ERROR_GENERIC,
CREATE_BRANCH_ERROR_WITH_CONTEXT,
- CREATE_BRANCH_SUCCESS_ALERT,
} from '~/jira_connect/branches/constants';
import createBranchMutation from '~/jira_connect/branches/graphql/mutations/create_branch.mutation.graphql';
@@ -74,10 +73,14 @@ describe('NewBranchForm', () => {
return mockApollo;
}
- function createComponent({ mockApollo } = {}) {
+ function createComponent({ mockApollo, provide } = {}) {
wrapper = shallowMount(NewBranchForm, {
localVue,
apolloProvider: mockApollo || createMockApolloProvider(),
+ provide: {
+ initialBranchName: '',
+ ...provide,
+ },
});
}
@@ -139,14 +142,8 @@ describe('NewBranchForm', () => {
await waitForPromises();
});
- it('displays a success message', () => {
- const alert = findAlert();
- expect(alert.exists()).toBe(true);
- expect(alert.text()).toBe(CREATE_BRANCH_SUCCESS_ALERT.message);
- expect(alert.props()).toMatchObject({
- title: CREATE_BRANCH_SUCCESS_ALERT.title,
- variant: 'success',
- });
+ it('emits `success` event', () => {
+ expect(wrapper.emitted('success')).toBeTruthy();
});
it('called `createBranch` mutation correctly', () => {
@@ -195,6 +192,15 @@ describe('NewBranchForm', () => {
});
});
+ describe('when `initialBranchName` is specified', () => {
+ it('sets value of branch name input to `initialBranchName` by default', () => {
+ const mockInitialBranchName = 'ap1-test-branch-name';
+
+ createComponent({ provide: { initialBranchName: mockInitialBranchName } });
+ expect(findInput().attributes('value')).toBe(mockInitialBranchName);
+ });
+ });
+
describe('error handling', () => {
describe.each`
component | componentName
diff --git a/spec/frontend/jira_connect/branches/pages/index_spec.js b/spec/frontend/jira_connect/branches/pages/index_spec.js
new file mode 100644
index 0000000000000..92976dd28dafa
--- /dev/null
+++ b/spec/frontend/jira_connect/branches/pages/index_spec.js
@@ -0,0 +1,65 @@
+import { GlEmptyState } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
+import NewBranchForm from '~/jira_connect/branches/components/new_branch_form.vue';
+import {
+ I18N_PAGE_TITLE_WITH_BRANCH_NAME,
+ I18N_PAGE_TITLE_DEFAULT,
+} from '~/jira_connect/branches/constants';
+import JiraConnectNewBranchPage from '~/jira_connect/branches/pages/index.vue';
+import { sprintf } from '~/locale';
+
+describe('NewBranchForm', () => {
+ let wrapper;
+
+ const findPageTitle = () => wrapper.find('h1');
+ const findNewBranchForm = () => wrapper.findComponent(NewBranchForm);
+ const findEmptyState = () => wrapper.findComponent(GlEmptyState);
+
+ function createComponent({ provide } = {}) {
+ wrapper = shallowMount(JiraConnectNewBranchPage, {
+ provide: {
+ initialBranchName: '',
+ successStateSvgPath: '',
+ ...provide,
+ },
+ });
+ }
+
+ afterEach(() => {
+ wrapper.destroy();
+ });
+
+ describe('page title', () => {
+ it.each`
+ initialBranchName | pageTitle
+ ${undefined} | ${I18N_PAGE_TITLE_DEFAULT}
+ ${'ap1-test-button'} | ${sprintf(I18N_PAGE_TITLE_WITH_BRANCH_NAME, { jiraIssue: 'ap1-test-button' })}
+ `(
+ 'sets page title to "$pageTitle" when initial branch name is "$initialBranchName"',
+ ({ initialBranchName, pageTitle }) => {
+ createComponent({ provide: { initialBranchName } });
+
+ expect(findPageTitle().text()).toBe(pageTitle);
+ },
+ );
+ });
+
+ it('renders NewBranchForm by default', () => {
+ createComponent();
+
+ expect(findNewBranchForm().exists()).toBe(true);
+ expect(findEmptyState().exists()).toBe(false);
+ });
+
+ describe('when `sucesss` event emitted from NewBranchForm', () => {
+ it('renders the success state', async () => {
+ createComponent();
+
+ const newBranchForm = findNewBranchForm();
+ await newBranchForm.vm.$emit('success');
+
+ expect(findNewBranchForm().exists()).toBe(false);
+ expect(findEmptyState().exists()).toBe(true);
+ });
+ });
+});
--
GitLab