diff --git a/app/assets/javascripts/design_management/components/design_description/description_form.vue b/app/assets/javascripts/design_management/components/design_description/description_form.vue index 6be643e88dcbb8b3529b26b711f70f33330082ae..03ebc73015a7d14b04fe543455a1329d136cbf96 100644 --- a/app/assets/javascripts/design_management/components/design_description/description_form.vue +++ b/app/assets/javascripts/design_management/components/design_description/description_form.vue @@ -1,7 +1,7 @@ <script> import { GlButton, GlFormGroup, GlAlert, GlTooltipDirective } from '@gitlab/ui'; import SafeHtml from '~/vue_shared/directives/safe_html'; -import { __, s__ } from '~/locale'; +import { s__ } from '~/locale'; import { helpPagePath } from '~/helpers/help_page_helper'; import MarkdownEditor from '~/vue_shared/components/markdown/markdown_editor.vue'; import { renderGFM } from '~/behaviors/markdown/render_gfm'; @@ -25,9 +25,9 @@ export default { GlTooltip: GlTooltipDirective, }, i18n: { - edit: __('Edit'), editDescription: s__('DesignManagement|Edit description'), - descriptionLabel: s__('DesignManagement|Design description'), + descriptionLabel: s__('DesignManagement|Description'), + addDescriptionLabel: s__('DesignManagement|Add a description'), }, formFieldProps: { id: 'design-description', @@ -66,6 +66,9 @@ export default { canUpdate() { return this.design.issue?.userPermissions?.updateDesign && !this.showEditor; }, + showAddDescriptionButton() { + return this.design.issue?.userPermissions?.updateDesign && !this.design.descriptionHtml; + }, }, watch: { 'design.descriptionHtml': { @@ -82,6 +85,11 @@ export default { this.showEditor = true; }, closeForm() { + // If description text is empty on cancel, + // restore the old description + if (!this.descriptionText) { + this.descriptionText = this.design.description; + } this.showEditor = false; }, async renderGFM() { @@ -160,6 +168,7 @@ export default { v-if="showEditor" class="design-description-form common-note-form" :label="$options.i18n.descriptionLabel" + label-class="gl-line-height-20! gl-font-lg!" > <div v-if="errorMessage" class="gl-pb-3"> <gl-alert variant="danger" @dismiss="errorMessage = null"> @@ -189,7 +198,7 @@ export default { :loading="isSubmitting" data-testid="save-description" @click="updateDesignDescription" - >{{ s__('DesignManagement|Save') }} + >{{ s__('DesignManagement|Save changes') }} </gl-button> <gl-button category="tertiary" class="gl-ml-3" data-testid="cancel" @click="closeForm" >{{ s__('DesignManagement|Cancel') }} @@ -197,40 +206,46 @@ export default { </div> </gl-form-group> <div v-else class="design-description-view"> - <div - class="design-description-header gl-display-flex gl-justify-content-space-between gl-mb-2" - > - <label class="gl-m-0"> - {{ $options.i18n.descriptionLabel }} - </label> - <gl-button - v-if="canUpdate" - v-gl-tooltip - class="gl-ml-auto" - size="small" - data-testid="edit-description" - :aria-label="$options.i18n.editDescription" - @click="startEditing" - > - {{ $options.i18n.edit }} - </gl-button> - </div> - <div - v-if="!design.descriptionHtml" - data-testid="design-description-none" - class="gl-text-secondary gl-mb-5" + <gl-button + v-if="showAddDescriptionButton" + class="gl-ml-auto gl-mb-5" + size="small" + data-testid="add-design-description" + :aria-label="$options.i18n.addDescriptionLabel" + @click="startEditing" > - {{ s__('DesignManagement|None') }} - </div> - <div v-else class="design-description js-task-list-container"> + {{ $options.i18n.addDescriptionLabel }} + </gl-button> + <template v-else> <div - ref="gfm-content" - v-safe-html="design.descriptionHtml" - class="md gl-mb-4" - data-testid="design-description-content" - @change="toggleCheckboxes" - ></div> - </div> + class="design-description-header gl-display-flex gl-justify-content-space-between gl-mb-2" + > + <h3 class="gl-line-height-20! gl-font-lg gl-m-0"> + {{ $options.i18n.descriptionLabel }} + </h3> + <gl-button + v-if="canUpdate" + v-gl-tooltip + class="gl-ml-auto" + size="small" + category="tertiary" + :aria-label="$options.i18n.editDescription" + :title="$options.i18n.editDescription" + data-testid="edit-description" + icon="pencil" + @click="startEditing" + /> + </div> + <div v-if="design.descriptionHtml" class="design-description js-task-list-container"> + <div + ref="gfm-content" + v-safe-html="design.descriptionHtml" + class="md gl-mb-4" + data-testid="design-description-content" + @change="toggleCheckboxes" + ></div> + </div> + </template> </div> </div> </template> diff --git a/app/assets/javascripts/design_management/components/design_sidebar.vue b/app/assets/javascripts/design_management/components/design_sidebar.vue index 0377bdb91009f9a86ef9cfdb0ecca41fde95f7da..80e7546d8ec9fc01fc931da485e7e2333b669927 100644 --- a/app/assets/javascripts/design_management/components/design_sidebar.vue +++ b/app/assets/javascripts/design_management/components/design_sidebar.vue @@ -108,6 +108,14 @@ export default { this.$emit('toggleResolvedComments', isExpanded); }, }, + showDescriptionForm() { + // user either has permission to add or update description, + // or the existing description should be shown read-only. + return ( + !this.isLoading && + (this.design.issue?.userPermissions?.updateDesign || Boolean(this.design.descriptionHtml)) + ); + }, }, mounted() { if (!this.isResolvedCommentsPopoverHidden && this.$refs.resolvedComments) { @@ -146,15 +154,20 @@ export default { <template #default> <div class="image-notes gl-h-full gl-pt-0" @click.self="handleSidebarClick"> <description-form - v-if="!isLoading" + v-if="showDescriptionForm" :design="design" :design-variables="designVariables" :markdown-preview-path="markdownPreviewPath" - class="gl-mt-4" + class="gl-my-5 gl-border-b" /> - <gl-skeleton-loader v-if="isLoading" /> + <div v-if="isLoading" class="gl-my-5"> + <gl-skeleton-loader /> + </div> <template v-else> - <h3 data-testid="unresolved-discussion-count" class="gl-line-height-20! gl-font-lg"> + <h3 + data-testid="unresolved-discussion-count" + class="gl-line-height-20! gl-font-lg gl-my-5" + > {{ unresolvedDiscussionsCount }} </h3> <gl-empty-state diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 289f6e72bb4a11f0ecf00028d66ba39c0f54c938..dae7330de2c0c58c9c14340b62a824c859d08511 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -18032,6 +18032,9 @@ msgid_plural "DesignManagement|The designs you tried uploading did not change." msgstr[0] "" msgstr[1] "" +msgid "DesignManagement|Add a description" +msgstr "" + msgid "DesignManagement|Adding a design with the same filename replaces the file in a new version." msgstr "" @@ -18089,6 +18092,9 @@ msgstr "" msgid "DesignManagement|Could not update discussion. Please try again." msgstr "" +msgid "DesignManagement|Description" +msgstr "" + msgid "DesignManagement|Deselect all" msgstr "" @@ -18125,9 +18131,6 @@ msgstr "" msgid "DesignManagement|Hide comments" msgstr "" -msgid "DesignManagement|None" -msgstr "" - msgid "DesignManagement|Requested design version does not exist. Showing latest version instead" msgstr "" @@ -18137,7 +18140,7 @@ msgstr "" msgid "DesignManagement|Resolved Comments" msgstr "" -msgid "DesignManagement|Save" +msgid "DesignManagement|Save changes" msgstr "" msgid "DesignManagement|Save comment" diff --git a/spec/frontend/design_management/components/design_description/description_form_spec.js b/spec/frontend/design_management/components/design_description/description_form_spec.js index 17a41acadc0c644d4c38a453300b09aafde3b679..905de12e1c01b0f04434c9aeaf5ac1cf196e5f4c 100644 --- a/spec/frontend/design_management/components/design_description/description_form_spec.js +++ b/spec/frontend/design_management/components/design_description/description_form_spec.js @@ -39,7 +39,6 @@ describe('Design description form', () => { const createComponent = ({ design = mockDesign, descriptionText = '', - showEditor = false, isSubmitting = false, designVariables = mockDesignVariables, designUpdateMutationHandler = mockDesignUpdateMutationHandler, @@ -56,7 +55,6 @@ describe('Design description form', () => { return { formFieldProps, descriptionText, - showEditor, isSubmitting, }; }, @@ -68,13 +66,14 @@ describe('Design description form', () => { }); const findDesignContent = () => wrapper.findByTestId('design-description-content'); - const findDesignNoneBlock = () => wrapper.findByTestId('design-description-none'); const findEditDescriptionButton = () => wrapper.findByTestId('edit-description'); const findSaveDescriptionButton = () => wrapper.findByTestId('save-description'); + const findCancelDescriptionButton = () => wrapper.findByTestId('cancel'); const findMarkdownEditor = () => wrapper.findComponent(MarkdownEditor); const findTextarea = () => wrapper.find('textarea'); const findCheckboxAtIndex = (index) => wrapper.findAll('input[type="checkbox"]').at(index); const findAlert = () => wrapper.findComponent(GlAlert); + const findAddDesignDescriptionButton = () => wrapper.findByTestId('add-design-description'); describe('user has updateDesign permission', () => { let trackingSpy; @@ -99,28 +98,22 @@ describe('Design description form', () => { expect(findEditDescriptionButton().exists()).toBe(true); }); - it('renders none when description is empty', () => { - createComponent({ design: designFactory({ description: '', descriptionHtml: '' }) }); - - expect(findDesignNoneBlock().text()).toEqual('None'); - }); + it('renders save button when editor is open', async () => { + createComponent(); - it('renders save button when editor is open', () => { - createComponent({ - design: designFactory({ description: '', descriptionHtml: '' }), - showEditor: true, - }); + await findEditDescriptionButton().vm.$emit('click'); expect(findSaveDescriptionButton().exists()).toBe(true); expect(findSaveDescriptionButton().attributes('disabled')).toBeUndefined(); }); - it('renders the markdown editor with default props', () => { + it('renders the markdown editor with default props', async () => { createComponent({ - showEditor: true, descriptionText: 'Test description', }); + await findEditDescriptionButton().vm.$emit('click'); + expect(findMarkdownEditor().exists()).toBe(true); expect(findMarkdownEditor().props()).toMatchObject({ value: 'Test description', @@ -137,6 +130,47 @@ describe('Design description form', () => { }); }); + it('renders add a description button when there is no description', () => { + createComponent({ + design: designFactory({ + description: '', + descriptionHtml: '', + }), + }); + + expect(findMarkdownEditor().exists()).toBe(false); + expect(findAddDesignDescriptionButton().exists()).toBe(true); + }); + + it('renders description form when add a description button is clicked', async () => { + createComponent({ + design: designFactory({ + description: '', + descriptionHtml: '', + }), + }); + + expect(findAddDesignDescriptionButton().exists()).toBe(true); + expect(findMarkdownEditor().exists()).toBe(false); + + await findAddDesignDescriptionButton().vm.$emit('click'); + + expect(findMarkdownEditor().exists()).toBe(true); + expect(findAddDesignDescriptionButton().exists()).toBe(false); + }); + + it('resets description text if empty when form is closed', async () => { + createComponent(); + + await findEditDescriptionButton().vm.$emit('click'); + + findMarkdownEditor().vm.$emit('input', ''); + + await findCancelDescriptionButton().vm.$emit('click'); + + expect(findDesignContent().text()).toEqual('Test description'); + }); + describe.each` isKeyEvent | assertionName | key | keyData ${true} | ${'Ctrl + Enter keypress'} | ${'ctrl'} | ${ctrlKey} @@ -154,10 +188,11 @@ describe('Design description form', () => { ); createComponent({ - showEditor: true, designUpdateMutationHandler: mockDesignUpdateResponseHandler, }); + await findEditDescriptionButton().vm.$emit('click'); + findMarkdownEditor().vm.$emit('input', 'Hello world'); if (isKeyEvent) { findTextarea().trigger('keydown.enter', keyData); @@ -192,11 +227,11 @@ describe('Design description form', () => { it('shows error message when mutation fails', async () => { const failureHandler = jest.fn().mockRejectedValue(new Error(errorMessage)); createComponent({ - showEditor: true, descriptionText: 'Hello world', designUpdateMutationHandler: failureHandler, }); + await findEditDescriptionButton().vm.$emit('click'); findMarkdownEditor().vm.$emit('input', 'Hello world'); findSaveDescriptionButton().vm.$emit('click'); @@ -289,14 +324,14 @@ describe('Design description form', () => { }); describe('user has no updateDesign permission', () => { - beforeEach(() => { - const designWithNoUpdateUserPermission = designFactory({ - updateDesign: false, + it('renders description content without edit button', () => { + createComponent({ + design: designFactory({ + updateDesign: false, + }), }); - createComponent({ design: designWithNoUpdateUserPermission }); - }); - it('does not render edit button', () => { + expect(findDesignContent().text()).toEqual('Test description'); expect(findEditDescriptionButton().exists()).toBe(false); }); }); diff --git a/spec/frontend/design_management/components/design_sidebar_spec.js b/spec/frontend/design_management/components/design_sidebar_spec.js index 6b3d9a63e9a83b12cacaf2c5cad7dbd8dd573955..ec72d6207934810985212f39687de9f5a756dc83 100644 --- a/spec/frontend/design_management/components/design_sidebar_spec.js +++ b/spec/frontend/design_management/components/design_sidebar_spec.js @@ -6,6 +6,7 @@ import DesignDiscussion from '~/design_management/components/design_notes/design import DesignNoteSignedOut from '~/design_management/components/design_notes/design_note_signed_out.vue'; import DesignSidebar from '~/design_management/components/design_sidebar.vue'; import DesignDisclosure from '~/design_management/components/design_disclosure.vue'; +import DescriptionForm from '~/design_management/components/design_description/description_form.vue'; import updateActiveDiscussionMutation from '~/design_management/graphql/mutations/update_active_discussion.mutation.graphql'; import design from '../mock_data/design'; @@ -44,6 +45,7 @@ describe('Design management design sidebar component', () => { const findUnresolvedDiscussions = () => wrapper.findAllByTestId('unresolved-discussion'); const findResolvedDiscussions = () => wrapper.findAllByTestId('resolved-discussion'); const findResolvedCommentsToggle = () => wrapper.findComponent(GlAccordionItem); + const findDescriptionForm = () => wrapper.findComponent(DescriptionForm); const findEmptyState = () => wrapper.findComponent(GlEmptyState); const findUnresolvedDiscussionsCount = () => wrapper.findByTestId('unresolved-discussion-count'); @@ -52,9 +54,10 @@ describe('Design management design sidebar component', () => { propsData: { design, resolvedDiscussionsExpanded: false, - markdownPreviewPath: '', + markdownPreviewPath: 'markdown/path', isLoading: false, designVariables: mockDesignVariables, + isOpen: true, ...props, }, mocks: { @@ -80,6 +83,53 @@ describe('Design management design sidebar component', () => { expect(findDisclosure().exists()).toBe(true); }); + describe('description form', () => { + it('does not render when loading', () => { + createComponent({ isLoading: true }); + + expect(findDescriptionForm().exists()).toBe(false); + }); + + it('renders with default props', () => { + createComponent(); + + expect(findDescriptionForm().props()).toMatchObject({ + design, + markdownPreviewPath: 'markdown/path', + designVariables: mockDesignVariables, + }); + }); + + it('renders when there is permission but description is empty', () => { + createComponent({ + design: { ...design, description: '', descriptionHtml: '' }, + }); + + expect(findDescriptionForm().exists()).toBe(true); + }); + + it('renders when there is description but no permission', () => { + createComponent({ + design: { ...design, issue: { userPermissions: { updateDesign: false } } }, + }); + + expect(findDescriptionForm().exists()).toBe(true); + }); + + it('does not render when there is no permission and description is empty', () => { + createComponent({ + design: { + ...design, + description: '', + descriptionHtml: '', + issue: { userPermissions: { updateDesign: false } }, + }, + }); + + expect(findDescriptionForm().exists()).toBe(false); + }); + }); + describe('when has no discussions', () => { beforeEach(() => { createComponent({