Commit 035560c5 authored by samdbeckham's avatar samdbeckham

Adds a few UX tweaks after a call with @andyvolpe

- 8px margins between vulnerability action buttons
- Sentence casing fixes on various parts of the dashboard
- Phrasing alignment between the action buttons and modal (dashboard)
- Renamed "Undo Dismissal" to "Revert Dismissal" (see above)
- Fixed the "more info" button color
parent 675741ab
...@@ -50,14 +50,14 @@ export default { ...@@ -50,14 +50,14 @@ export default {
title: s__('Security Reports|At this time, the security dashboard only supports SAST.'), title: s__('Security Reports|At this time, the security dashboard only supports SAST.'),
content: ` content: `
<a <a
title="${s__('Security Reports|Security Dashboard Documentation')}" title="${s__('Security Reports|Security dashboard documentation')}"
href="${this.dashboardDocumentation}" href="${this.dashboardDocumentation}"
target="_blank" target="_blank"
rel="noopener rel="noopener
noreferrer" noreferrer"
> >
<span class="vertical-align-middle">${s__( <span class="vertical-align-middle">${s__(
'Security Reports|Security Dashboard Documentation', 'Security Reports|Security dashboard documentation',
)}</span> )}</span>
${spriteIcon('external-link', 's16 vertical-align-middle')} ${spriteIcon('external-link', 's16 vertical-align-middle')}
</a> </a>
...@@ -78,7 +78,7 @@ export default { ...@@ -78,7 +78,7 @@ export default {
'fetchVulnerabilitiesCount', 'fetchVulnerabilitiesCount',
'createIssue', 'createIssue',
'dismissVulnerability', 'dismissVulnerability',
'undoDismissal', 'revertDismissal',
]), ]),
}, },
}; };
...@@ -118,7 +118,7 @@ export default { ...@@ -118,7 +118,7 @@ export default {
:can-create-feedback-permission="true" :can-create-feedback-permission="true"
@createNewIssue="createIssue({ vulnerability: modal.vulnerability })" @createNewIssue="createIssue({ vulnerability: modal.vulnerability })"
@dismissIssue="dismissVulnerability({ vulnerability: modal.vulnerability })" @dismissIssue="dismissVulnerability({ vulnerability: modal.vulnerability })"
@revertDismissIssue="undoDismissal({ vulnerability: modal.vulnerability })" @revertDismissIssue="revertDismissal({ vulnerability: modal.vulnerability })"
/> />
</div> </div>
</template> </template>
...@@ -42,7 +42,7 @@ export default { ...@@ -42,7 +42,7 @@ export default {
'openModal', 'openModal',
'createIssue', 'createIssue',
'dismissVulnerability', 'dismissVulnerability',
'undoDismissal', 'revertDismissal',
]), ]),
handleCreateIssue() { handleCreateIssue() {
const { vulnerability } = this; const { vulnerability } = this;
...@@ -52,9 +52,9 @@ export default { ...@@ -52,9 +52,9 @@ export default {
const { vulnerability } = this; const { vulnerability } = this;
this.dismissVulnerability({ vulnerability, flashError: true }); this.dismissVulnerability({ vulnerability, flashError: true });
}, },
handleUndoDismissal() { handleRevertDismissal() {
const { vulnerability } = this; const { vulnerability } = this;
this.undoDismissal({ vulnerability, flashError: true }); this.revertDismissal({ vulnerability, flashError: true });
}, },
}, },
}; };
...@@ -67,7 +67,7 @@ export default { ...@@ -67,7 +67,7 @@ export default {
v-gl-tooltip v-gl-tooltip
:aria-label="s__('Security Reports|More info')" :aria-label="s__('Security Reports|More info')"
:title="s__('Security Reports|More info')" :title="s__('Security Reports|More info')"
class="btn btn-secondary js-more-info" class="btn btn-secondary hint js-more-info"
type="button" type="button"
@click="openModal({ vulnerability })" @click="openModal({ vulnerability })"
> >
...@@ -77,10 +77,10 @@ export default { ...@@ -77,10 +77,10 @@ export default {
v-if="canCreateIssue" v-if="canCreateIssue"
key="create-issue" key="create-issue"
v-gl-tooltip v-gl-tooltip
:aria-label="s__('Security Reports|New Issue')" :aria-label="s__('Security Reports|Create issue')"
:loading="isCreatingIssue" :loading="isCreatingIssue"
:title="s__('Security Reports|New Issue')" :title="s__('Security Reports|Create issue')"
container-class="btn btn-inverted btn-success js-create-issue" container-class="btn btn-inverted btn-success prepend-left-8 js-create-issue"
type="button" type="button"
@click="handleCreateIssue" @click="handleCreateIssue"
> >
...@@ -89,21 +89,21 @@ export default { ...@@ -89,21 +89,21 @@ export default {
<template v-if="canDismissVulnerability"> <template v-if="canDismissVulnerability">
<loading-button <loading-button
v-if="isDismissed" v-if="isDismissed"
key="undo-dismissal" key="revert-dismissal"
:label="s__('Security Reports|Undo Dismissal')" :label="s__('Security Reports|Revert dismissal')"
:loading="isDismissingVulnerability" :loading="isDismissingVulnerability"
container-class="btn btn-inverted btn-warning js-undo-dismissal" container-class="btn btn-inverted btn-warning prepend-left-8 js-revert-dismissal"
type="button" type="button"
@click="handleUndoDismissal" @click="handleRevertDismissal"
/> />
<loading-button <loading-button
v-else v-else
key="dismiss-vulnerability" key="dismiss-vulnerability"
v-gl-tooltip v-gl-tooltip
:aria-label="s__('Security Reports|Dismiss Vulnerability')" :aria-label="s__('Security Reports|Dismiss vulnerability')"
:loading="isDismissingVulnerability" :loading="isDismissingVulnerability"
:title="s__('Security Reports|Dismiss Vulnerability')" :title="s__('Security Reports|Dismiss vulnerability')"
container-class="btn btn-inverted btn-warning js-dismiss-vulnerability" container-class="btn btn-inverted btn-warning prepend-left-8 js-dismiss-vulnerability"
type="button" type="button"
@click="handleDismissVulnerability" @click="handleDismissVulnerability"
> >
......
...@@ -167,37 +167,37 @@ export const receiveDismissVulnerabilityError = ({ commit }, { flashError }) => ...@@ -167,37 +167,37 @@ export const receiveDismissVulnerabilityError = ({ commit }, { flashError }) =>
} }
}; };
export const undoDismissal = ({ dispatch }, { vulnerability, flashError }) => { export const revertDismissal = ({ dispatch }, { vulnerability, flashError }) => {
const { vulnerability_feedback_url, dismissal_feedback } = vulnerability; const { vulnerability_feedback_url, dismissal_feedback } = vulnerability;
// eslint-disable-next-line camelcase // eslint-disable-next-line camelcase
const url = `${vulnerability_feedback_url}/${dismissal_feedback.id}`; const url = `${vulnerability_feedback_url}/${dismissal_feedback.id}`;
dispatch('requestUndoDismissal'); dispatch('requestRevertDismissal');
axios axios
.delete(url) .delete(url)
.then(() => { .then(() => {
const { id } = vulnerability; const { id } = vulnerability;
dispatch('receiveUndoDismissalSuccess', { id }); dispatch('receiveRevertDismissalSuccess', { id });
}) })
.catch(() => { .catch(() => {
dispatch('receiveUndoDismissalError', { flashError }); dispatch('receiveRevertDismissalError', { flashError });
}); });
}; };
export const requestUndoDismissal = ({ commit }) => { export const requestRevertDismissal = ({ commit }) => {
commit(types.REQUEST_UNDO_DISMISSAL); commit(types.REQUEST_REVERT_DISMISSAL);
}; };
export const receiveUndoDismissalSuccess = ({ commit }, payload) => { export const receiveRevertDismissalSuccess = ({ commit }, payload) => {
commit(types.RECEIVE_UNDO_DISMISSAL_SUCCESS, payload); commit(types.RECEIVE_REVERT_DISMISSAL_SUCCESS, payload);
}; };
export const receiveUndoDismissalError = ({ commit }, { flashError }) => { export const receiveRevertDismissalError = ({ commit }, { flashError }) => {
commit(types.RECEIVE_UNDO_DISMISSAL_ERROR); commit(types.RECEIVE_REVERT_DISMISSAL_ERROR);
if (flashError) { if (flashError) {
createFlash( createFlash(
s__('Security Reports|There was an error undoing this dismissal.'), s__('Security Reports|There was an error reverting this dismissal.'),
'alert', 'alert',
document.querySelector('.ci-table'), document.querySelector('.ci-table'),
); );
......
...@@ -18,6 +18,6 @@ export const REQUEST_DISMISS_VULNERABILITY = 'REQUEST_DISMISS_VULNERABILITY'; ...@@ -18,6 +18,6 @@ export const REQUEST_DISMISS_VULNERABILITY = 'REQUEST_DISMISS_VULNERABILITY';
export const RECEIVE_DISMISS_VULNERABILITY_SUCCESS = 'RECEIVE_DISMISS_VULNERABILITY_SUCCESS'; export const RECEIVE_DISMISS_VULNERABILITY_SUCCESS = 'RECEIVE_DISMISS_VULNERABILITY_SUCCESS';
export const RECEIVE_DISMISS_VULNERABILITY_ERROR = 'RECEIVE_DISMISS_VULNERABILITY_ERROR'; export const RECEIVE_DISMISS_VULNERABILITY_ERROR = 'RECEIVE_DISMISS_VULNERABILITY_ERROR';
export const REQUEST_UNDO_DISMISSAL = 'REQUEST_UNDO_DISMISSAL'; export const REQUEST_REVERT_DISMISSAL = 'REQUEST_REVERT_DISMISSAL';
export const RECEIVE_UNDO_DISMISSAL_SUCCESS = 'RECEIVE_UNDO_DISMISSAL_SUCCESS'; export const RECEIVE_REVERT_DISMISSAL_SUCCESS = 'RECEIVE_REVERT_DISMISSAL_SUCCESS';
export const RECEIVE_UNDO_DISMISSAL_ERROR = 'RECEIVE_UNDO_DISMISSAL_ERROR'; export const RECEIVE_REVERT_DISMISSAL_ERROR = 'RECEIVE_REVERT_DISMISSAL_ERROR';
...@@ -101,25 +101,25 @@ export default { ...@@ -101,25 +101,25 @@ export default {
s__('Security Reports|There was an error dismissing the vulnerability.'), s__('Security Reports|There was an error dismissing the vulnerability.'),
); );
}, },
[types.REQUEST_UNDO_DISMISSAL](state) { [types.REQUEST_REVERT_DISMISSAL](state) {
state.isDismissingVulnerability = true; state.isDismissingVulnerability = true;
Vue.set(state.modal, 'isDismissingVulnerability', true); Vue.set(state.modal, 'isDismissingVulnerability', true);
Vue.set(state.modal, 'error', null); Vue.set(state.modal, 'error', null);
}, },
[types.RECEIVE_UNDO_DISMISSAL_SUCCESS](state, payload) { [types.RECEIVE_REVERT_DISMISSAL_SUCCESS](state, payload) {
const vulnerability = state.vulnerabilities.find(vuln => vuln.id === payload.id); const vulnerability = state.vulnerabilities.find(vuln => vuln.id === payload.id);
vulnerability.dismissal_feedback = null; vulnerability.dismissal_feedback = null;
state.isDismissingVulnerability = false; state.isDismissingVulnerability = false;
Vue.set(state.modal, 'isDismissingVulnerability', false); Vue.set(state.modal, 'isDismissingVulnerability', false);
Vue.set(state.modal.vulnerability, 'isDismissed', false); Vue.set(state.modal.vulnerability, 'isDismissed', false);
}, },
[types.RECEIVE_UNDO_DISMISSAL_ERROR](state) { [types.RECEIVE_REVERT_DISMISSAL_ERROR](state) {
state.isDismissingVulnerability = false; state.isDismissingVulnerability = false;
Vue.set(state.modal, 'isDismissingVulnerability', false); Vue.set(state.modal, 'isDismissingVulnerability', false);
Vue.set( Vue.set(
state.modal, state.modal,
'error', 'error',
s__('Security Reports|There was an error undoing the dismissal.'), s__('Security Reports|There was an error reverting the dismissal.'),
); );
}, },
}; };
---
title: UX improvements for the group security dashboard
merge_request: 8217
author:
type: other
...@@ -124,10 +124,12 @@ describe('Security Dashboard Action Buttons', () => { ...@@ -124,10 +124,12 @@ describe('Security Dashboard Action Buttons', () => {
}); });
}); });
describe('with a vulnerbility that has been dismissed', () => { describe('with a vulnerability that has been dismissed', () => {
beforeEach(() => { beforeEach(() => {
props = { props = {
vulnerability: mockDataVulnerabilities[2], vulnerability: mockDataVulnerabilities[2],
canDismissVulnerability: true,
isDismissed: true,
}; };
vm = mountComponentWithStore(Component, { store, props }); vm = mountComponentWithStore(Component, { store, props });
}); });
...@@ -136,12 +138,16 @@ describe('Security Dashboard Action Buttons', () => { ...@@ -136,12 +138,16 @@ describe('Security Dashboard Action Buttons', () => {
vm.$destroy(); vm.$destroy();
}); });
it('should only render one button', () => { it('should render two buttons', () => {
expect(vm.$el.querySelectorAll('.btn')).toHaveLength(1); expect(vm.$el.querySelectorAll('.btn')).toHaveLength(2);
}); });
it('should not render the dismiss vulnerability button', () => { it('should not render the dismiss vulnerability button', () => {
expect(vm.$el.querySelector('.js-dismiss-vulnerability')).toBeNull(); expect(vm.$el.querySelector('.js-dismiss-vulnerability')).toBeNull();
}); });
it('should render the revert dismissal button', () => {
expect(vm.$el.querySelector('.js-revert-dismissal')).not.toBeNull();
});
}); });
}); });
...@@ -524,8 +524,8 @@ describe('vulnerability dismissal', () => { ...@@ -524,8 +524,8 @@ describe('vulnerability dismissal', () => {
}); });
}); });
describe('undo vulnerability dismissal', () => { describe('revert vulnerability dismissal', () => {
describe('undoDismissal', () => { describe('revertDismissal', () => {
const vulnerability = mockDataVulnerabilities[2]; const vulnerability = mockDataVulnerabilities[2];
const url = `${vulnerability.vulnerability_feedback_url}/${ const url = `${vulnerability.vulnerability_feedback_url}/${
vulnerability.dismissal_feedback.id vulnerability.dismissal_feedback.id
...@@ -547,13 +547,13 @@ describe('undo vulnerability dismissal', () => { ...@@ -547,13 +547,13 @@ describe('undo vulnerability dismissal', () => {
it('should dispatch the request and success actions', done => { it('should dispatch the request and success actions', done => {
testAction( testAction(
actions.undoDismissal, actions.revertDismissal,
{ vulnerability }, { vulnerability },
{}, {},
[], [],
[ [
{ type: 'requestUndoDismissal' }, { type: 'requestRevertDismissal' },
{ type: 'receiveUndoDismissalSuccess', payload: { id: vulnerability.id } }, { type: 'receiveRevertDismissalSuccess', payload: { id: vulnerability.id } },
], ],
done, done,
); );
...@@ -569,13 +569,13 @@ describe('undo vulnerability dismissal', () => { ...@@ -569,13 +569,13 @@ describe('undo vulnerability dismissal', () => {
const flashError = false; const flashError = false;
testAction( testAction(
actions.undoDismissal, actions.revertDismissal,
{ vulnerability, flashError }, { vulnerability, flashError },
{}, {},
[], [],
[ [
{ type: 'requestUndoDismissal' }, { type: 'requestRevertDismissal' },
{ type: 'receiveUndoDismissalError', payload: { flashError: false } }, { type: 'receiveRevertDismissalError', payload: { flashError: false } },
], ],
done, done,
); );
...@@ -583,18 +583,18 @@ describe('undo vulnerability dismissal', () => { ...@@ -583,18 +583,18 @@ describe('undo vulnerability dismissal', () => {
}); });
}); });
describe('receiveUndoDismissalSuccess', () => { describe('receiveRevertDismissalSuccess', () => {
it('should commit the success mutation', done => { it('should commit the success mutation', done => {
const state = initialState; const state = initialState;
const data = mockDataVulnerabilities[0]; const data = mockDataVulnerabilities[0];
testAction( testAction(
actions.receiveUndoDismissalSuccess, actions.receiveRevertDismissalSuccess,
{ data }, { data },
state, state,
[ [
{ {
type: types.RECEIVE_UNDO_DISMISSAL_SUCCESS, type: types.RECEIVE_REVERT_DISMISSAL_SUCCESS,
payload: { data }, payload: { data },
}, },
], ],
...@@ -604,30 +604,30 @@ describe('undo vulnerability dismissal', () => { ...@@ -604,30 +604,30 @@ describe('undo vulnerability dismissal', () => {
}); });
}); });
describe('receiveUndoDismissalError', () => { describe('receiveRevertDismissalError', () => {
it('should commit the error mutation', done => { it('should commit the error mutation', done => {
const state = initialState; const state = initialState;
testAction( testAction(
actions.receiveUndoDismissalError, actions.receiveRevertDismissalError,
{}, {},
state, state,
[{ type: types.RECEIVE_UNDO_DISMISSAL_ERROR }], [{ type: types.RECEIVE_REVERT_DISMISSAL_ERROR }],
[], [],
done, done,
); );
}); });
}); });
describe('requestUndoDismissal', () => { describe('requestRevertDismissal', () => {
it('should commit the request mutation', done => { it('should commit the request mutation', done => {
const state = initialState; const state = initialState;
testAction( testAction(
actions.requestUndoDismissal, actions.requestRevertDismissal,
{}, {},
state, state,
[{ type: types.REQUEST_UNDO_DISMISSAL }], [{ type: types.REQUEST_REVERT_DISMISSAL }],
[], [],
done, done,
); );
......
...@@ -342,12 +342,12 @@ describe('vulnerabilities module mutations', () => { ...@@ -342,12 +342,12 @@ describe('vulnerabilities module mutations', () => {
}); });
}); });
describe('REQUEST_UNDO_DISMISSAL', () => { describe('REQUEST_REVERT_DISMISSAL', () => {
let state; let state;
beforeEach(() => { beforeEach(() => {
state = createState(); state = createState();
mutations[types.REQUEST_UNDO_DISMISSAL](state); mutations[types.REQUEST_REVERT_DISMISSAL](state);
}); });
it('should set isDismissingVulnerability to true', () => { it('should set isDismissingVulnerability to true', () => {
...@@ -363,7 +363,7 @@ describe('vulnerabilities module mutations', () => { ...@@ -363,7 +363,7 @@ describe('vulnerabilities module mutations', () => {
}); });
}); });
describe('RECEIVE_UNDO_DISMISSAL_SUCCESS', () => { describe('RECEIVE_REVERT_DISMISSAL_SUCCESS', () => {
let state; let state;
let payload; let payload;
let vulnerability; let vulnerability;
...@@ -373,7 +373,7 @@ describe('vulnerabilities module mutations', () => { ...@@ -373,7 +373,7 @@ describe('vulnerabilities module mutations', () => {
state.vulnerabilities = mockData; state.vulnerabilities = mockData;
[vulnerability] = mockData; [vulnerability] = mockData;
payload = { id: vulnerability.id }; payload = { id: vulnerability.id };
mutations[types.RECEIVE_UNDO_DISMISSAL_SUCCESS](state, payload); mutations[types.RECEIVE_REVERT_DISMISSAL_SUCCESS](state, payload);
}); });
it('should set the dismissal feedback on the passed vulnerability', () => { it('should set the dismissal feedback on the passed vulnerability', () => {
...@@ -393,12 +393,12 @@ describe('vulnerabilities module mutations', () => { ...@@ -393,12 +393,12 @@ describe('vulnerabilities module mutations', () => {
}); });
}); });
describe('RECEIVE_UNDO_DISMISSAL_ERROR', () => { describe('RECEIVE_REVERT_DISMISSAL_ERROR', () => {
let state; let state;
beforeEach(() => { beforeEach(() => {
state = createState(); state = createState();
mutations[types.RECEIVE_UNDO_DISMISSAL_ERROR](state); mutations[types.RECEIVE_REVERT_DISMISSAL_ERROR](state);
}); });
it('should set isDismissingVulnerability to false', () => { it('should set isDismissingVulnerability to false', () => {
...@@ -410,7 +410,7 @@ describe('vulnerabilities module mutations', () => { ...@@ -410,7 +410,7 @@ describe('vulnerabilities module mutations', () => {
}); });
it('should set the error state on the modal', () => { it('should set the error state on the modal', () => {
expect(state.modal.error).toEqual('There was an error undoing the dismissal.'); expect(state.modal.error).toEqual('There was an error reverting the dismissal.');
}); });
}); });
}); });
...@@ -7157,16 +7157,19 @@ msgstr "" ...@@ -7157,16 +7157,19 @@ msgstr ""
msgid "Security Reports|At this time, the security dashboard only supports SAST." msgid "Security Reports|At this time, the security dashboard only supports SAST."
msgstr "" msgstr ""
msgid "Security Reports|Dismiss Vulnerability" msgid "Security Reports|Create issue"
msgstr ""
msgid "Security Reports|Dismiss vulnerability"
msgstr "" msgstr ""
msgid "Security Reports|More info" msgid "Security Reports|More info"
msgstr "" msgstr ""
msgid "Security Reports|New Issue" msgid "Security Reports|Revert dismissal"
msgstr "" msgstr ""
msgid "Security Reports|Security Dashboard Documentation" msgid "Security Reports|Security dashboard documentation"
msgstr "" msgstr ""
msgid "Security Reports|There was an error creating the issue." msgid "Security Reports|There was an error creating the issue."
...@@ -7175,13 +7178,10 @@ msgstr "" ...@@ -7175,13 +7178,10 @@ msgstr ""
msgid "Security Reports|There was an error dismissing the vulnerability." msgid "Security Reports|There was an error dismissing the vulnerability."
msgstr "" msgstr ""
msgid "Security Reports|There was an error undoing the dismissal." msgid "Security Reports|There was an error reverting the dismissal."
msgstr ""
msgid "Security Reports|There was an error undoing this dismissal."
msgstr "" msgstr ""
msgid "Security Reports|Undo Dismissal" msgid "Security Reports|There was an error reverting this dismissal."
msgstr "" msgstr ""
msgid "SecurityDashboard| The security dashboard displays the latest security report. Use it to find and fix vulnerabilities." msgid "SecurityDashboard| The security dashboard displays the latest security report. Use it to find and fix vulnerabilities."
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment