diff --git a/app/assets/javascripts/snippets/components/snippet_header.vue b/app/assets/javascripts/snippets/components/snippet_header.vue index c6507f27c07c0024884367ecfebf52601385c97b..8f6b98c4587198ddedff04837f6c8d3204a4a0dd 100644 --- a/app/assets/javascripts/snippets/components/snippet_header.vue +++ b/app/assets/javascripts/snippets/components/snippet_header.vue @@ -6,8 +6,9 @@ import { GlModal, GlAlert, GlLoadingIcon, - GlDropdown, - GlDropdownItem, + GlDisclosureDropdown, + GlDisclosureDropdownGroup, + GlDisclosureDropdownItem, GlButton, GlTooltipDirective, } from '@gitlab/ui'; @@ -29,6 +30,7 @@ export const i18n = { { spammable_titlecase: __('Snippet') }, ), snippetSpamFailure: s__('Snippets|Error with Akismet. Please check the logs for more info.'), + snippetAction: s__('Snippets|Snippet actions'), }; export default { @@ -39,8 +41,9 @@ export default { GlModal, GlAlert, GlLoadingIcon, - GlDropdown, - GlDropdownItem, + GlDisclosureDropdown, + GlDisclosureDropdownGroup, + GlDisclosureDropdownItem, TimeAgoTooltip, GlButton, }, @@ -79,6 +82,7 @@ export default { errorMessage: '', canCreateSnippet: false, isDeleteModalVisible: false, + isDropdownShown: false, }; }, computed: { @@ -90,44 +94,52 @@ export default { ? __('Authored %{timeago} by %{author}') : __('Authored %{timeago}'); }, - personalSnippetActions() { - return [ - { - condition: this.snippet.userPermissions.updateSnippet, - text: __('Edit'), - href: this.editLink, - disabled: this.snippetHasBinary, - title: this.snippetHasBinary - ? __('Snippets with non-text files can only be edited via Git.') - : undefined, + editItem() { + return { + text: __('Edit'), + href: this.editLink, + disabled: this.snippetHasBinary, + title: this.snippetHasBinary + ? __('Snippets with non-text files can only be edited via Git.') + : undefined, + extraAttrs: { + class: 'gl-sm-display-none!', }, - { - condition: this.snippet.userPermissions.adminSnippet, - text: __('Delete'), - click: this.showDeleteModal, - variant: 'danger', - category: 'secondary', - }, - { - condition: this.canCreateSnippet, - text: __('New snippet'), - href: this.snippet.project - ? joinPaths(this.snippet.project.webUrl, '-/snippets/new') - : joinPaths('/', gon.relative_url_root, '/-/snippets/new'), - variant: 'confirm', - category: 'secondary', - }, - { - condition: this.canReportSpam && !isEmpty(this.reportAbusePath), - text: __('Submit as spam'), - click: this.submitAsSpam, - title: __('Submit as spam'), - loading: this.isSubmittingSpam, + }; + }, + canReportSpaCheck() { + return this.canReportSpam && !isEmpty(this.reportAbusePath); + }, + spamItem() { + return { + text: __('Submit as spam'), + action: () => this.submitAsSpam(), + }; + }, + deleteItem() { + return { + text: __('Delete'), + action: () => this.showDeleteModal(), + extraAttrs: { + class: 'gl-text-red-500!', }, - ]; + }; + }, + newSnippetItem() { + return { + text: __('New snippet'), + href: this.snippet.project + ? joinPaths(this.snippet.project.webUrl, '-/snippets/new') + : joinPaths('/', gon.relative_url_root, '/-/snippets/new'), + }; }, hasPersonalSnippetActions() { - return Boolean(this.personalSnippetActions.filter(({ condition }) => condition).length); + return ( + this.snippet.userPermissions.updateSnippet || + this.canCreateSnippet || + this.snippet.userPermissions.adminSnippet || + this.canReportSpaCheck + ); }, editLink() { return `${this.snippet.webUrl}/edit`; @@ -157,6 +169,9 @@ export default { return 'earth'; } }, + showDropdownTooltip() { + return !this.isDropdownShown ? this.$options.i18n.snippetAction : ''; + }, }, methods: { redirectToSnippets() { @@ -170,6 +185,12 @@ export default { showDeleteModal() { this.isDeleteModalVisible = true; }, + onShowDropdown() { + this.isDropdownShown = true; + }, + onHideDropdown() { + this.isDropdownShown = false; + }, deleteSnippet() { this.isLoading = true; this.$apollo @@ -257,47 +278,62 @@ export default { </div> </div> - <div v-if="hasPersonalSnippetActions" class="detail-page-header-actions gl-align-self-start"> - <div class="d-none d-sm-flex"> - <template v-for="(action, index) in personalSnippetActions"> - <div - v-if="action.condition" - :key="index" - v-gl-tooltip - :title="action.title" - class="d-inline-block" - :class="{ 'gl-ml-3': index > 0 }" - > + <div + v-if="hasPersonalSnippetActions" + class="detail-page-header-actions gl-display-flex gl-align-self-center gl-gap-3 gl-relative" + > + <gl-button + v-if="snippet.userPermissions.updateSnippet" + :href="editItem.href" + :title="editItem.title" + :disabled="editItem.disabled" + class="gl-display-none gl-sm-display-inline-block" + data-testid="snippet-action-button" + :data-qa-action="editItem.text" + > + {{ editItem.text }} + </gl-button> + + <gl-disclosure-dropdown + data-testid="snippets-more-actions-dropdown" + @shown="onShowDropdown" + @hidden="onHideDropdown" + > + <template #toggle> + <div class="gl-w-full gl-min-h-7"> <gl-button - :disabled="action.disabled" - :loading="action.loading" - :variant="action.variant" - :category="action.category" - :class="action.cssClass" - :href="action.href" - data-testid="snippet-action-button" - :data-qa-action="action.text" - @click="action.click ? action.click() : undefined" - >{{ action.text }}</gl-button + class="gl-sm-display-none! gl-new-dropdown-toggle gl-absolute gl-top-0 gl-left-0 gl-w-full" + button-text-classes="gl-display-flex gl-justify-content-space-between gl-w-full" + category="secondary" + tabindex="0" > + <span>{{ $options.i18n.snippetAction }}</span> + <gl-icon class="dropdown-chevron" name="chevron-down" /> + </gl-button> + <gl-button + v-gl-tooltip="showDropdownTooltip" + class="gl-display-none gl-sm-display-flex! gl-new-dropdown-toggle gl-new-dropdown-icon-only gl-new-dropdown-toggle-no-caret" + category="tertiary" + icon="ellipsis_v" + :aria-label="$options.i18n.snippetAction" + tabindex="0" + data-testid="snippets-more-actions-dropdown-toggle" + /> </div> </template> - </div> - <div class="d-block d-sm-none dropdown"> - <gl-dropdown :text="__('Options')" block> - <template v-for="(action, index) in personalSnippetActions"> - <gl-dropdown-item - v-if="action.condition" - :key="index" - :disabled="action.disabled" - :title="action.title" - :href="action.href" - @click="action.click ? action.click() : undefined" - >{{ action.text }}</gl-dropdown-item - > - </template> - </gl-dropdown> - </div> + <gl-disclosure-dropdown-item + v-if="snippet.userPermissions.updateSnippet" + :item="editItem" + /> + <gl-disclosure-dropdown-item v-if="canCreateSnippet" :item="newSnippetItem" /> + <gl-disclosure-dropdown-group bordered> + <gl-disclosure-dropdown-item v-if="canReportSpaCheck" :item="spamItem" /> + <gl-disclosure-dropdown-item + v-if="snippet.userPermissions.adminSnippet" + :item="deleteItem" + /> + </gl-disclosure-dropdown-group> + </gl-disclosure-dropdown> </div> <gl-modal diff --git a/app/assets/stylesheets/components/detail_page.scss b/app/assets/stylesheets/components/detail_page.scss index 95987dbb08b2074387b3c4bf146100b7d46cc251..570dbbe079bf3f4387a57249a9b0a729feee75bb 100644 --- a/app/assets/stylesheets/components/detail_page.scss +++ b/app/assets/stylesheets/components/detail_page.scss @@ -37,11 +37,6 @@ .detail-page-header-actions { flex: 0 0 auto; - - @include media-breakpoint-down(sm) { - width: 100%; - margin-top: 10px; - } } .detail-page-header-meta { diff --git a/app/views/shared/empty_states/_snippets.html.haml b/app/views/shared/empty_states/_snippets.html.haml index 800cfe8b0d151f1d5830f9181f185a81c4c81e2e..6f38514d8d26dd238f7d6cc6ed401c550d809859 100644 --- a/app/views/shared/empty_states/_snippets.html.haml +++ b/app/views/shared/empty_states/_snippets.html.haml @@ -12,7 +12,7 @@ = s_('SnippetsEmptyState|Store, share, and embed small pieces of code and text.') .gl-mt-3< - if button_path - = link_button_to s_('SnippetsEmptyState|New snippet'), button_path, title: s_('SnippetsEmptyState|New snippet'), id: 'new_snippet_link', data: { testid: 'create-first-snippet-link' }, variant: :confirm + = link_button_to s_('SnippetsEmptyState|New snippet'), button_path, title: s_('SnippetsEmptyState|New snippet'), data: { testid: 'create-first-snippet-link' }, variant: :confirm = link_button_to s_('SnippetsEmptyState|Documentation'), help_page_path('user/snippets'), title: s_('SnippetsEmptyState|Documentation') - else %h4.gl-text-center= s_('SnippetsEmptyState|There are no snippets to show.') diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d4bc0d4dc4e2a35f7624be4af811b45a6b31bc4a..ff6354219780b8ca9bcfcef402674984e67d0cf6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -48328,6 +48328,9 @@ msgstr "" msgid "Snippets|Files" msgstr "" +msgid "Snippets|Snippet actions" +msgstr "" + msgid "Snippets|Snippets are limited to %{total} files." msgstr "" diff --git a/qa/qa/page/component/snippet.rb b/qa/qa/page/component/snippet.rb index 91c915877e26ec5f2f543f92ec510c9450dbddcc..a710cf0dbb3dff76d3c5ffb41d0333a7f6dfa2c7 100644 --- a/qa/qa/page/component/snippet.rb +++ b/qa/qa/page/component/snippet.rb @@ -172,7 +172,8 @@ def click_edit_button end def click_delete_button - click_element('snippet-action-button', action: 'Delete') + click_element('snippets-more-actions-dropdown-toggle') + click_button('Delete') click_element('delete-snippet-button') # wait for the page to reload after deletion wait_until(reload: false) do diff --git a/spec/features/projects/snippets/user_deletes_snippet_spec.rb b/spec/features/projects/snippets/user_deletes_snippet_spec.rb index 6a825fe18de55a886b11e5fa2caee22dc7dcdb17..a3f39d6623109dbb362a0ef5a8264fa57c829ffd 100644 --- a/spec/features/projects/snippets/user_deletes_snippet_spec.rb +++ b/spec/features/projects/snippets/user_deletes_snippet_spec.rb @@ -3,8 +3,10 @@ require 'spec_helper' RSpec.describe 'Projects > Snippets > User deletes a snippet', :js, feature_category: :source_code_management do + include Spec::Support::Helpers::ModalHelpers + let(:project) { create(:project) } - let!(:snippet) { create(:project_snippet, :repository, project: project, author: user) } + let!(:snippet) { create(:project_snippet, project: project, author: user) } let(:user) { create(:user) } before do @@ -17,12 +19,22 @@ it 'deletes a snippet' do expect(page).to have_content(snippet.title) - click_button('Delete') - click_button('Delete snippet') + click_on 'Snippet actions' + + page.within(find_by_testid('snippets-more-actions-dropdown')) do + click_on 'Delete' + end + + within_modal do + click_button 'Delete snippet' + end + wait_for_requests # This assertion also confirms we did not end up on an error page - expect(page).to have_selector('#new_snippet_link') + expect(current_url).to end_with(project_snippets_path(project)) + expect(page).to have_link('New snippet') + expect(page).not_to have_content(snippet.title) expect(project.snippets.length).to eq(0) end end diff --git a/spec/features/snippets/user_deletes_snippet_spec.rb b/spec/features/snippets/user_deletes_snippet_spec.rb index 3c4c41b01813e58fa02c95fdcfd407061784ce26..fe17a7b29c59a9b6dc2f3fb8199fb47894253da0 100644 --- a/spec/features/snippets/user_deletes_snippet_spec.rb +++ b/spec/features/snippets/user_deletes_snippet_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe 'User deletes snippet', :js, feature_category: :source_code_management do + include Spec::Support::Helpers::ModalHelpers + let(:user) { create(:user) } let(:content) { 'puts "test"' } let(:snippet) { create(:personal_snippet, :repository, :public, content: content, author: user) } @@ -16,10 +18,19 @@ it 'deletes the snippet' do expect(page).to have_content(snippet.title) - click_button('Delete') - click_button('Delete snippet') + click_on 'Snippet actions' + + page.within(find_by_testid('snippets-more-actions-dropdown')) do + click_on 'Delete' + end + + within_modal do + click_button 'Delete snippet' + end + wait_for_requests + expect(page).to have_link('New snippet') expect(page).not_to have_content(snippet.title) end end diff --git a/spec/frontend/snippets/components/snippet_header_spec.js b/spec/frontend/snippets/components/snippet_header_spec.js index 1eb5de70e4b8aa7d32e3b061ed1a4f94dd896e2e..73a69ccda9b6d8b8e88b69bcdca404cc9dbe73c4 100644 --- a/spec/frontend/snippets/components/snippet_header_spec.js +++ b/spec/frontend/snippets/components/snippet_header_spec.js @@ -1,8 +1,14 @@ -import { GlModal, GlButton, GlDropdown } from '@gitlab/ui'; -import { mount } from '@vue/test-utils'; +import { + GlModal, + GlButton, + GlDisclosureDropdown, + GlDisclosureDropdownGroup, + GlDisclosureDropdownItem, +} from '@gitlab/ui'; import VueApollo from 'vue-apollo'; import MockAdapter from 'axios-mock-adapter'; import Vue, { nextTick } from 'vue'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import { useMockLocationHelper } from 'helpers/mock_window_location_helper'; import waitForPromises from 'helpers/wait_for_promises'; @@ -60,7 +66,7 @@ describe('Snippet header component', () => { [DeleteSnippetMutation, deleteSnippetMock], ]); - wrapper = mount(SnippetHeader, { + wrapper = mountExtended(SnippetHeader, { provide: { reportAbusePath, canReportSpam, @@ -73,33 +79,24 @@ describe('Snippet header component', () => { }, stubs: { GlEmoji, + GlButton, + GlDisclosureDropdown, + GlDisclosureDropdownGroup, + GlDisclosureDropdownItem, }, apolloProvider: mockApollo, }); } const findAuthorEmoji = () => wrapper.findComponent(GlEmoji); - const findAuthoredMessage = () => wrapper.find('[data-testid="authored-message"]').text(); - const findAuthorUsername = () => wrapper.find('[data-testid="authored-username"]'); - const findButtons = () => wrapper.findAllComponents(GlButton); - const findButtonsAsModel = () => - findButtons().wrappers.map((x) => ({ - text: x.text(), - href: x.attributes('href'), - category: x.props('category'), - variant: x.props('variant'), - disabled: x.props('disabled'), - })); - const findResponsiveDropdown = () => wrapper.findComponent(GlDropdown); - // We can't search by component here since we are full mounting and the attributes are applied to a child of the GlDropdownItem - const findResponsiveDropdownItems = () => findResponsiveDropdown().findAll('[role="menuitem"]'); - const findResponsiveDropdownItemsAsModel = () => - findResponsiveDropdownItems().wrappers.map((x) => ({ - disabled: x.attributes('disabled'), - href: x.attributes('href'), - title: x.attributes('title'), - text: x.text(), - })); + const findAuthoredMessage = () => wrapper.findByTestId('authored-message').text(); + const findAuthorUsername = () => wrapper.findByTestId('authored-username'); + const findEditButton = () => wrapper.findByTestId('snippet-action-button'); + const findDropdown = () => wrapper.findComponent(GlDisclosureDropdown); + const findDropdownItems = () => wrapper.findAllComponents(GlDisclosureDropdownItem); + const findDropdownItemAt = (i) => findDropdownItems().at(i).props('item'); + const findSpamAction = () => wrapper.findByText('Submit as spam'); + const findDeleteAction = () => wrapper.findByText('Delete'); const findDeleteModal = () => wrapper.findComponent(GlModal); beforeEach(() => { @@ -185,54 +182,27 @@ describe('Snippet header component', () => { expect(text).toBe('Authored 1 month ago'); }); - it('renders a action buttons', () => { + it('renders an edit button on sm and up screens', () => { createComponent(); - expect(findButtonsAsModel()).toEqual([ - { - category: 'primary', - disabled: false, - href: `${snippet.webUrl}/edit`, - text: 'Edit', - variant: 'default', - }, - { - category: 'secondary', - disabled: false, - text: 'Delete', - variant: 'danger', - }, - { - category: 'primary', - disabled: false, - text: 'Submit as spam', - variant: 'default', - }, - ]); + expect(findEditButton().attributes('href')).toEqual(`${snippet.webUrl}/edit`); + expect(findEditButton().attributes('class')).toContain('gl-display-none'); + expect(findEditButton().attributes('class')).toContain('gl-sm-display-inline-block'); }); - it('renders responsive dropdown for action buttons', () => { + it('renders dropdown for action buttons', () => { createComponent(); - expect(findResponsiveDropdownItemsAsModel()).toEqual([ - { - href: `${snippet.webUrl}/edit`, - text: 'Edit', - }, - { - text: 'Delete', - }, - { - text: 'Submit as spam', - title: 'Submit as spam', - }, - ]); + expect(findDropdownItemAt(0).text).toBe('Edit'); + expect(findDropdownItemAt(0).href).toBe(`${snippet.webUrl}/edit`); + expect(findDropdownItemAt(1).text).toBe('Submit as spam'); + expect(findDropdownItemAt(2).text).toBe('Delete'); }); it.each` permissions | buttons ${{ adminSnippet: false, updateSnippet: false }} | ${['Submit as spam']} - ${{ adminSnippet: true, updateSnippet: false }} | ${['Delete', 'Submit as spam']} + ${{ adminSnippet: true, updateSnippet: false }} | ${['Submit as spam', 'Delete']} ${{ adminSnippet: false, updateSnippet: true }} | ${['Edit', 'Submit as spam']} `('with permissions ($permissions), renders buttons ($buttons)', ({ permissions, buttons }) => { createComponent({ @@ -241,10 +211,10 @@ describe('Snippet header component', () => { }, }); - expect(findButtonsAsModel().map((x) => x.text)).toEqual(buttons); + expect(findDropdownItems().wrappers.map((x) => x.props('item').text)).toEqual(buttons); }); - it('with canCreateSnippet permission, renders create button', async () => { + it('with canCreateSnippet permission, renders new snippet button', async () => { createComponent({ canCreateProjectSnippetMock: jest .fn() @@ -256,17 +226,8 @@ describe('Snippet header component', () => { await waitForPromises(); - expect(findButtonsAsModel()).toEqual( - expect.arrayContaining([ - { - category: 'secondary', - disabled: false, - href: `/foo/-/snippets/new`, - text: 'New snippet', - variant: 'confirm', - }, - ]), - ); + expect(findDropdownItemAt(1).text).toBe('New snippet'); + expect(findDropdownItemAt(1).href).toBe('/foo/-/snippets/new'); }); describe('submit snippet as spam', () => { @@ -281,9 +242,9 @@ describe('Snippet header component', () => { `( 'renders a "$variant" alert message with "$text" message for a request with a "$request" response', async ({ request, variant, text }) => { - const submitAsSpamBtn = findButtons().at(2); mock.onPost(reportAbusePath).reply(request); - submitAsSpamBtn.trigger('click'); + findDropdown().trigger('click'); + findSpamAction().trigger('click'); await waitForPromises(); expect(createAlert).toHaveBeenLastCalledWith({ @@ -309,11 +270,11 @@ describe('Snippet header component', () => { }); it('does not show any action buttons', () => { - expect(findButtons()).toHaveLength(0); + expect(findEditButton().exists()).toBe(false); }); - it('does not show responsive action dropdown', () => { - expect(findResponsiveDropdown().exists()).toBe(false); + it('does not show action dropdown', () => { + expect(findDropdown().exists()).toBe(false); }); }); @@ -339,13 +300,16 @@ describe('Snippet header component', () => { describe('Delete mutation', () => { const deleteSnippet = async () => { // Click delete action - findButtons().at(1).trigger('click'); + findDropdown().trigger('click'); + findDeleteAction().trigger('click'); + await nextTick(); expect(findDeleteModal().props().visible).toBe(true); // Click delete button in delete modal document.querySelector('[data-testid="delete-snippet-button"').click(); + await waitForPromises(); }; diff --git a/spec/support/shared_examples/features/snippets_shared_examples.rb b/spec/support/shared_examples/features/snippets_shared_examples.rb index 0f830fa125a56fb31c7a733e93fc0069a8a5b13a..7fca64b1a90d856f9ec13f4348dee3febf23d7d3 100644 --- a/spec/support/shared_examples/features/snippets_shared_examples.rb +++ b/spec/support/shared_examples/features/snippets_shared_examples.rb @@ -54,13 +54,17 @@ RSpec.shared_examples 'does not show New Snippet button' do specify do expect(page).to have_link(text: "$#{snippet.id}") + expect(page).not_to have_selector('[data-testid="snippets-more-actions-dropdown"]') expect(page).not_to have_link('New snippet') end end RSpec.shared_examples 'does show New Snippet button' do specify do + find_by_testid('snippets-more-actions-dropdown-toggle').click + expect(page).to have_link(text: "$#{snippet.id}") + expect(page).to have_selector('[data-testid="snippets-more-actions-dropdown"]') expect(page).to have_link('New snippet') end end