Added diff suggestion popover

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/56523
This commit is contained in:
Phil Hughes 2019-06-14 14:01:24 +01:00
parent 577832598f
commit d2fd6bd510
No known key found for this signature in database
GPG key ID: 32245528C52E0F9F
25 changed files with 269 additions and 79 deletions

View file

@ -69,6 +69,16 @@ export default {
required: false,
default: false,
},
dismissEndpoint: {
type: String,
required: false,
default: '',
},
showSuggestPopover: {
type: Boolean,
required: false,
default: false,
},
},
data() {
const treeWidth =
@ -141,7 +151,12 @@ export default {
showTreeList: 'adjustView',
},
mounted() {
this.setBaseConfig({ endpoint: this.endpoint, projectPath: this.projectPath });
this.setBaseConfig({
endpoint: this.endpoint,
projectPath: this.projectPath,
dismissEndpoint: this.dismissEndpoint,
showSuggestPopover: this.showSuggestPopover,
});
if (this.shouldShow) {
this.fetchData();

View file

@ -42,6 +42,7 @@ export default {
noteableData: state => state.notes.noteableData,
diffViewType: state => state.diffs.diffViewType,
}),
...mapState('diffs', ['showSuggestPopover']),
...mapGetters('diffs', ['getDiffFileByHash']),
...mapGetters([
'isLoggedIn',
@ -80,7 +81,12 @@ export default {
}
},
methods: {
...mapActions('diffs', ['cancelCommentForm', 'assignDiscussionsToDiff', 'saveDiffDiscussion']),
...mapActions('diffs', [
'cancelCommentForm',
'assignDiscussionsToDiff',
'saveDiffDiscussion',
'setSuggestPopoverDismissed',
]),
handleCancelCommentForm(shouldConfirm, isDirty) {
if (shouldConfirm && isDirty) {
const msg = s__('Notes|Are you sure you want to cancel creating this comment?');
@ -125,11 +131,13 @@ export default {
:line="line"
:help-page-path="helpPagePath"
:diff-file="diffFile"
:show-suggest-popover="showSuggestPopover"
save-button-title="Comment"
class="diff-comment-form"
@handleFormUpdateAddToReview="addToReview"
@cancelForm="handleCancelCommentForm"
@handleFormUpdate="handleSaveNote"
@handleSuggestDismissed="setSuggestPopoverDismissed"
/>
</div>
</template>

View file

@ -72,6 +72,8 @@ export default function initDiffsApp(store) {
currentUser: JSON.parse(dataset.currentUserData) || {},
changesEmptyStateIllustration: dataset.changesEmptyStateIllustration,
isFluidLayout: parseBoolean(dataset.isFluidLayout),
dismissEndpoint: dataset.dismissEndpoint,
showSuggestPopover: parseBoolean(dataset.showSuggestPopover),
};
},
computed: {
@ -99,6 +101,8 @@ export default function initDiffsApp(store) {
shouldShow: this.activeTab === 'diffs',
changesEmptyStateIllustration: this.changesEmptyStateIllustration,
isFluidLayout: this.isFluidLayout,
dismissEndpoint: this.dismissEndpoint,
showSuggestPopover: this.showSuggestPopover,
},
});
},

View file

@ -36,8 +36,8 @@ import {
import { diffViewerModes } from '~/ide/constants';
export const setBaseConfig = ({ commit }, options) => {
const { endpoint, projectPath } = options;
commit(types.SET_BASE_CONFIG, { endpoint, projectPath });
const { endpoint, projectPath, dismissEndpoint, showSuggestPopover } = options;
commit(types.SET_BASE_CONFIG, { endpoint, projectPath, dismissEndpoint, showSuggestPopover });
};
export const fetchDiffFiles = ({ state, commit }) => {
@ -455,5 +455,17 @@ export const toggleFullDiff = ({ dispatch, getters, state }, filePath) => {
export const setFileCollapsed = ({ commit }, { filePath, collapsed }) =>
commit(types.SET_FILE_COLLAPSED, { filePath, collapsed });
export const setSuggestPopoverDismissed = ({ commit, state }) =>
axios
.post(state.dismissEndpoint, {
feature_name: 'suggest_popover_dismissed',
})
.then(() => {
commit(types.SET_SHOW_SUGGEST_POPOVER);
})
.catch(() => {
createFlash(s__('MergeRequest|Error dismissing suggestion popover. Please try again.'));
});
// prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {};

View file

@ -28,4 +28,6 @@ export default () => ({
renderTreeList: true,
showWhitespace: true,
fileFinderVisible: false,
dismissEndpoint: '',
showSuggestPopover: true,
});

View file

@ -33,3 +33,5 @@ export const SET_HIDDEN_VIEW_DIFF_FILE_LINES = 'SET_HIDDEN_VIEW_DIFF_FILE_LINES'
export const SET_CURRENT_VIEW_DIFF_FILE_LINES = 'SET_CURRENT_VIEW_DIFF_FILE_LINES';
export const ADD_CURRENT_VIEW_DIFF_FILE_LINES = 'ADD_CURRENT_VIEW_DIFF_FILE_LINES';
export const TOGGLE_DIFF_FILE_RENDERING_MORE = 'TOGGLE_DIFF_FILE_RENDERING_MORE';
export const SET_SHOW_SUGGEST_POPOVER = 'SET_SHOW_SUGGEST_POPOVER';

View file

@ -11,8 +11,8 @@ import * as types from './mutation_types';
export default {
[types.SET_BASE_CONFIG](state, options) {
const { endpoint, projectPath } = options;
Object.assign(state, { endpoint, projectPath });
const { endpoint, projectPath, dismissEndpoint, showSuggestPopover } = options;
Object.assign(state, { endpoint, projectPath, dismissEndpoint, showSuggestPopover });
},
[types.SET_LOADING](state, isLoading) {
@ -302,4 +302,7 @@ export default {
file.renderingLines = !file.renderingLines;
},
[types.SET_SHOW_SUGGEST_POPOVER](state) {
state.showSuggestPopover = false;
},
};

View file

@ -77,6 +77,11 @@ export default {
required: false,
default: '',
},
showSuggestPopover: {
type: Boolean,
required: false,
default: false,
},
},
data() {
let updatedNoteBody = this.noteBody;
@ -247,6 +252,8 @@ export default {
:can-suggest="canSuggest"
:add-spacing-classes="false"
:help-page-path="helpPagePath"
:show-suggest-popover="showSuggestPopover"
@handleSuggestDismissed="() => $emit('handleSuggestDismissed')"
>
<textarea
id="note_note"
@ -303,7 +310,7 @@ export default {
{{ __('Add comment now') }}
</button>
<button
class="btn btn-cancel note-edit-cancel js-close-discussion-note-form"
class="btn note-edit-cancel js-close-discussion-note-form"
type="button"
@click="cancelHandler()"
>

View file

@ -67,6 +67,11 @@ export default {
required: false,
default: '',
},
showSuggestPopover: {
type: Boolean,
required: false,
default: false,
},
},
data() {
return {
@ -194,8 +199,10 @@ export default {
:preview-markdown="previewMarkdown"
:line-content="lineContent"
:can-suggest="canSuggest"
:show-suggest-popover="showSuggestPopover"
@preview-markdown="showPreviewTab"
@write-markdown="showWriteTab"
@handleSuggestDismissed="() => $emit('handleSuggestDismissed')"
/>
<div v-show="!previewMarkdown" class="md-write-holder">
<div class="zen-backdrop">

View file

@ -1,6 +1,6 @@
<script>
import $ from 'jquery';
import { GlTooltipDirective } from '@gitlab/ui';
import { GlPopover, GlButton, GlTooltipDirective } from '@gitlab/ui';
import ToolbarButton from './toolbar_button.vue';
import Icon from '../icon.vue';
@ -8,6 +8,8 @@ export default {
components: {
ToolbarButton,
Icon,
GlPopover,
GlButton,
},
directives: {
GlTooltip: GlTooltipDirective,
@ -27,6 +29,11 @@ export default {
required: false,
default: true,
},
showSuggestPopover: {
type: Boolean,
required: false,
default: false,
},
},
computed: {
mdTable() {
@ -70,6 +77,9 @@ export default {
this.$emit('write-markdown');
},
handleSuggestDismissed() {
this.$emit('handleSuggestDismissed');
},
},
};
</script>
@ -93,66 +103,92 @@ export default {
</button>
</li>
<li :class="{ active: !previewMarkdown }" class="md-header-toolbar">
<toolbar-button tag="**" :button-title="__('Add bold text')" icon="bold" />
<toolbar-button tag="*" :button-title="__('Add italic text')" icon="italic" />
<toolbar-button
:prepend="true"
tag="> "
:button-title="__('Insert a quote')"
icon="quote"
/>
<toolbar-button tag="`" tag-block="```" :button-title="__('Insert code')" icon="code" />
<toolbar-button
tag="[{text}](url)"
tag-select="url"
:button-title="__('Add a link')"
icon="link"
/>
<toolbar-button
:prepend="true"
tag="* "
:button-title="__('Add a bullet list')"
icon="list-bulleted"
/>
<toolbar-button
:prepend="true"
tag="1. "
:button-title="__('Add a numbered list')"
icon="list-numbered"
/>
<toolbar-button
:prepend="true"
tag="* [ ] "
:button-title="__('Add a task list')"
icon="task-done"
/>
<toolbar-button
:tag="mdTable"
:prepend="true"
:button-title="__('Add a table')"
icon="table"
/>
<toolbar-button
v-if="canSuggest"
:tag="mdSuggestion"
:prepend="true"
:button-title="__('Insert suggestion')"
:cursor-offset="4"
:tag-content="lineContent"
icon="doc-code"
class="qa-suggestion-btn"
/>
<button
v-gl-tooltip
:aria-label="__('Go full screen')"
class="toolbar-btn toolbar-fullscreen-btn js-zen-enter"
data-container="body"
tabindex="-1"
:title="__('Go full screen')"
type="button"
>
<icon name="screen-full" />
</button>
<div class="d-inline-block">
<toolbar-button tag="**" :button-title="__('Add bold text')" icon="bold" />
<toolbar-button tag="*" :button-title="__('Add italic text')" icon="italic" />
<toolbar-button
:prepend="true"
tag="> "
:button-title="__('Insert a quote')"
icon="quote"
/>
</div>
<div class="d-inline-block ml-md-2 ml-0">
<template v-if="canSuggest">
<toolbar-button
ref="suggestButton"
:tag="mdSuggestion"
:prepend="true"
:button-title="__('Insert suggestion')"
:cursor-offset="4"
:tag-content="lineContent"
icon="doc-code"
class="qa-suggestion-btn"
@click="handleSuggestDismissed"
/>
<gl-popover
v-if="showSuggestPopover"
:target="() => $refs.suggestButton"
:css-classes="['diff-suggest-popover']"
placement="bottom"
:show="showSuggestPopover"
>
<strong>{{ __('New! Suggest changes directly') }}</strong>
<p class="mb-2">
{{ __('Suggest code changes which are immediately applied. Try it out!') }}
</p>
<gl-button variant="primary" size="sm" @click="handleSuggestDismissed">
{{ __('Got it') }}
</gl-button>
</gl-popover>
</template>
<toolbar-button tag="`" tag-block="```" :button-title="__('Insert code')" icon="code" />
<toolbar-button
tag="[{text}](url)"
tag-select="url"
:button-title="__('Add a link')"
icon="link"
/>
</div>
<div class="d-inline-block ml-md-2 ml-0">
<toolbar-button
:prepend="true"
tag="* "
:button-title="__('Add a bullet list')"
icon="list-bulleted"
/>
<toolbar-button
:prepend="true"
tag="1. "
:button-title="__('Add a numbered list')"
icon="list-numbered"
/>
<toolbar-button
:prepend="true"
tag="* [ ] "
:button-title="__('Add a task list')"
icon="task-done"
/>
<toolbar-button
:tag="mdTable"
:prepend="true"
:button-title="__('Add a table')"
icon="table"
/>
</div>
<div class="d-inline-block ml-md-2 ml-0">
<button
v-gl-tooltip
:aria-label="__('Go full screen')"
class="toolbar-btn toolbar-fullscreen-btn js-zen-enter"
data-container="body"
tabindex="-1"
:title="__('Go full screen')"
type="button"
>
<icon name="screen-full" />
</button>
</div>
</li>
</ul>
</div>

View file

@ -66,6 +66,7 @@ export default {
class="toolbar-btn js-md"
tabindex="-1"
data-container="body"
@click="() => $emit('click')"
>
<icon :name="icon" />
</button>

View file

@ -154,11 +154,9 @@
}
.toolbar-fullscreen-btn {
margin-left: $gl-padding;
margin-right: -5px;
@include media-breakpoint-down(xs) {
margin-left: 0;
margin-right: 0;
}
}

View file

@ -14,7 +14,7 @@
position: -webkit-sticky;
position: sticky;
top: $mr-file-header-top;
z-index: 102;
z-index: 220;
&::before {
content: '';
@ -1122,3 +1122,15 @@ table.code {
outline: 0;
}
}
.diff-suggest-popover {
&.popover {
width: 250px;
min-width: 250px;
z-index: 210;
}
.popover-header {
display: none;
}
}

View file

@ -3,6 +3,7 @@
module UserCalloutsHelper
GKE_CLUSTER_INTEGRATION = 'gke_cluster_integration'.freeze
GCP_SIGNUP_OFFER = 'gcp_signup_offer'.freeze
SUGGEST_POPOVER_DISMISSED = 'suggest_popover_dismissed'.freeze
def show_gke_cluster_integration_callout?(project)
can?(current_user, :create_cluster, project) &&
@ -20,6 +21,10 @@ module UserCalloutsHelper
def render_dashboard_gold_trial(user)
end
def show_suggest_popover?
!user_dismissed?(SUGGEST_POPOVER_DISMISSED)
end
private
def user_dismissed?(feature_name)

View file

@ -10,7 +10,8 @@ module UserCalloutEnums
{
gke_cluster_integration: 1,
gcp_signup_offer: 2,
cluster_security_warning: 3
cluster_security_warning: 3,
suggest_popover_dismissed: 4
}
end
end

View file

@ -80,7 +80,9 @@
current_user_data: UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json,
project_path: project_path(@merge_request.project),
changes_empty_state_illustration: image_path('illustrations/merge_request_changes_empty.svg'),
is_fluid_layout: fluid_layout.to_s } }
is_fluid_layout: fluid_layout.to_s,
dismiss_endpoint: user_callouts_path,
show_suggest_popover: show_suggest_popover?.to_s } }
.mr-loading-status
= spinner

View file

@ -0,0 +1,5 @@
---
title: Added diff suggestion feature discovery popover
merge_request:
author:
type: added

View file

@ -4761,6 +4761,9 @@ msgstr ""
msgid "Google authentication is not %{link_to_documentation}. Ask your GitLab administrator if you want to use this service."
msgstr ""
msgid "Got it"
msgstr ""
msgid "Got it!"
msgstr ""
@ -6153,6 +6156,9 @@ msgstr ""
msgid "MergeRequest| %{paragraphStart}changed the description %{descriptionChangedTimes} times %{timeDifferenceMinutes}%{paragraphEnd}"
msgstr ""
msgid "MergeRequest|Error dismissing suggestion popover. Please try again."
msgstr ""
msgid "MergeRequest|Error loading full diff. Please try again."
msgstr ""
@ -6509,6 +6515,9 @@ msgstr ""
msgid "New users set to external"
msgstr ""
msgid "New! Suggest changes directly"
msgstr ""
msgid "New..."
msgstr ""
@ -9643,6 +9652,9 @@ msgstr ""
msgid "Successfully unlocked"
msgstr ""
msgid "Suggest code changes which are immediately applied. Try it out!"
msgstr ""
msgid "Suggested change"
msgstr ""

View file

@ -32,7 +32,7 @@ describe 'Issue markdown toolbar', :js do
find('.js-main-target-form #note-body')
page.evaluate_script('document.querySelectorAll(".js-main-target-form #note-body")[0].setSelectionRange(4, 50)')
find('.toolbar-btn:nth-child(2)').click
all('.toolbar-btn')[1].click
expect(find('#note-body')[:value]).to eq("test\n*underline*\n")
end

View file

@ -141,7 +141,7 @@ describe 'Merge request > User posts notes', :js do
page.within('.current-note-edit-form') do
expect(find('#note_note').value).to eq('This is the new content')
find('.js-md:first-child').click
first('.js-md').click
expect(find('#note_note').value).to eq('This is the new content****')
end
end

View file

@ -28,6 +28,18 @@ describe 'User comments on a diff', :js do
end
context 'single suggestion note' do
it 'hides suggestion popover' do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']"))
expect(page).to have_selector('.diff-suggest-popover')
page.within('.diff-suggest-popover') do
click_button 'Got it'
end
expect(page).not_to have_selector('.diff-suggest-popover')
end
it 'suggestion is presented' do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']"))

View file

@ -37,6 +37,8 @@ describe('diffs/components/app', () => {
projectPath: 'namespace/project',
currentUser: {},
changesEmptyStateIllustration: '',
dismissEndpoint: '',
showSuggestPopover: true,
...props,
},
store,

View file

@ -37,6 +37,7 @@ import actions, {
toggleFullDiff,
setFileCollapsed,
setExpandedDiffLines,
setSuggestPopoverDismissed,
} from '~/diffs/store/actions';
import eventHub from '~/notes/event_hub';
import * as types from '~/diffs/store/mutation_types';
@ -68,12 +69,19 @@ describe('DiffsStoreActions', () => {
it('should set given endpoint and project path', done => {
const endpoint = '/diffs/set/endpoint';
const projectPath = '/root/project';
const dismissEndpoint = '/-/user_callouts';
const showSuggestPopover = false;
testAction(
setBaseConfig,
{ endpoint, projectPath },
{ endpoint: '', projectPath: '' },
[{ type: types.SET_BASE_CONFIG, payload: { endpoint, projectPath } }],
{ endpoint, projectPath, dismissEndpoint, showSuggestPopover },
{ endpoint: '', projectPath: '', dismissEndpoint: '', showSuggestPopover: true },
[
{
type: types.SET_BASE_CONFIG,
payload: { endpoint, projectPath, dismissEndpoint, showSuggestPopover },
},
],
[],
done,
);
@ -1080,4 +1088,30 @@ describe('DiffsStoreActions', () => {
);
});
});
describe('setSuggestPopoverDismissed', () => {
it('commits SET_SHOW_SUGGEST_POPOVER', done => {
const state = { dismissEndpoint: `${gl.TEST_HOST}/-/user_callouts` };
const mock = new MockAdapter(axios);
mock.onPost(state.dismissEndpoint).reply(200, {});
spyOn(axios, 'post').and.callThrough();
testAction(
setSuggestPopoverDismissed,
null,
state,
[{ type: types.SET_SHOW_SUGGEST_POPOVER }],
[],
() => {
expect(axios.post).toHaveBeenCalledWith(state.dismissEndpoint, {
feature_name: 'suggest_popover_dismissed',
});
mock.restore();
done();
},
);
});
});
});

View file

@ -850,4 +850,14 @@ describe('DiffsStoreMutations', () => {
expect(file.renderingLines).toBe(false);
});
});
describe('SET_SHOW_SUGGEST_POPOVER', () => {
it('sets showSuggestPopover to false', () => {
const state = { showSuggestPopover: true };
mutations[types.SET_SHOW_SUGGEST_POPOVER](state);
expect(state.showSuggestPopover).toBe(false);
});
});
});

View file

@ -22,13 +22,13 @@ describe('Markdown field header component', () => {
'Add bold text',
'Add italic text',
'Insert a quote',
'Insert suggestion',
'Insert code',
'Add a link',
'Add a bullet list',
'Add a numbered list',
'Add a task list',
'Add a table',
'Insert suggestion',
'Go full screen',
];
const elements = vm.$el.querySelectorAll('.toolbar-btn');