Commit b02efd8c authored by Jarek Ostrowski's avatar Jarek Ostrowski

Update sort discussion to dropdown

MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41201
parent 93fc192d
<script> <script>
import $ from 'jquery';
import { mapGetters, mapActions } from 'vuex'; import { mapGetters, mapActions } from 'vuex';
import { GlIcon } from '@gitlab/ui'; import { GlDropdown, GlDropdownItem, GlDropdownDivider } from '@gitlab/ui';
import { getLocationHash, doesHashExistInUrl } from '../../lib/utils/url_utility'; import { getLocationHash, doesHashExistInUrl } from '../../lib/utils/url_utility';
import { import {
DISCUSSION_FILTERS_DEFAULT_VALUE, DISCUSSION_FILTERS_DEFAULT_VALUE,
...@@ -14,7 +13,9 @@ import notesEventHub from '../event_hub'; ...@@ -14,7 +13,9 @@ import notesEventHub from '../event_hub';
export default { export default {
components: { components: {
GlIcon, GlDropdown,
GlDropdownItem,
GlDropdownDivider,
}, },
props: { props: {
filters: { filters: {
...@@ -66,9 +67,6 @@ export default { ...@@ -66,9 +67,6 @@ export default {
selectFilter(value, persistFilter = true) { selectFilter(value, persistFilter = true) {
const filter = parseInt(value, 10); const filter = parseInt(value, 10);
// close dropdown
this.toggleDropdown();
if (filter === this.currentValue) return; if (filter === this.currentValue) return;
this.currentValue = filter; this.currentValue = filter;
this.filterDiscussion({ this.filterDiscussion({
...@@ -78,9 +76,6 @@ export default { ...@@ -78,9 +76,6 @@ export default {
}); });
this.toggleCommentsForm(); this.toggleCommentsForm();
}, },
toggleDropdown() {
$(this.$refs.dropdownToggle).dropdown('toggle');
},
toggleCommentsForm() { toggleCommentsForm() {
this.setCommentsDisabled(this.currentValue === HISTORY_ONLY_FILTER_VALUE); this.setCommentsDisabled(this.currentValue === HISTORY_ONLY_FILTER_VALUE);
}, },
...@@ -92,7 +87,6 @@ export default { ...@@ -92,7 +87,6 @@ export default {
if (/^note_/.test(hash) && this.currentValue !== DISCUSSION_FILTERS_DEFAULT_VALUE) { if (/^note_/.test(hash) && this.currentValue !== DISCUSSION_FILTERS_DEFAULT_VALUE) {
this.selectFilter(this.defaultValue, false); this.selectFilter(this.defaultValue, false);
this.toggleDropdown(); // close dropdown
this.setTargetNoteHash(hash); this.setTargetNoteHash(hash);
} }
}, },
...@@ -109,43 +103,24 @@ export default { ...@@ -109,43 +103,24 @@ export default {
</script> </script>
<template> <template>
<div <gl-dropdown
v-if="displayFilters" v-if="displayFilters"
class="discussion-filter-container js-discussion-filter-container d-inline-block align-bottom full-width-mobile"
>
<button
id="discussion-filter-dropdown" id="discussion-filter-dropdown"
ref="dropdownToggle" class="gl-mr-3 full-width-mobile discussion-filter-container js-discussion-filter-container qa-discussion-filter"
class="btn btn-sm qa-discussion-filter" :text="currentFilter.title"
data-toggle="dropdown"
aria-expanded="false"
> >
{{ currentFilter.title }} <gl-icon name="chevron-down" /> <div v-for="filter in filters" :key="filter.value" class="dropdown-item-wrapper">
</button> <gl-dropdown-item
<div :is-check-item="true"
ref="dropdownMenu" :is-checked="filter.value === currentValue"
class="dropdown-menu dropdown-menu-selectable dropdown-menu-right"
aria-labelledby="discussion-filter-dropdown"
>
<div class="dropdown-content">
<ul>
<li
v-for="filter in filters"
:key="filter.value"
:data-filter-type="filterType(filter.value)"
>
<button
:class="{ 'is-active': filter.value === currentValue }" :class="{ 'is-active': filter.value === currentValue }"
:data-filter-type="filterType(filter.value)"
class="qa-filter-options" class="qa-filter-options"
type="button" @click.prevent="selectFilter(filter.value)"
@click="selectFilter(filter.value)"
> >
{{ filter.title }} {{ filter.title }}
</button> </gl-dropdown-item>
<div v-if="filter.value === defaultValue" class="dropdown-divider"></div> <gl-dropdown-divider v-if="filter.value === defaultValue" />
</li>
</ul>
</div>
</div>
</div> </div>
</gl-dropdown>
</template> </template>
gs
<script> <script>
import { GlIcon } from '@gitlab/ui'; import { GlDropdown, GlDropdownItem } from '@gitlab/ui';
import { mapActions, mapGetters } from 'vuex'; import { mapActions, mapGetters } from 'vuex';
import { __ } from '~/locale'; import { __ } from '~/locale';
import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue';
...@@ -15,7 +14,8 @@ const SORT_OPTIONS = [ ...@@ -15,7 +14,8 @@ const SORT_OPTIONS = [
export default { export default {
SORT_OPTIONS, SORT_OPTIONS,
components: { components: {
GlIcon, GlDropdown,
GlDropdownItem,
LocalStorageSync, LocalStorageSync,
}, },
mixins: [Tracking.mixin()], mixins: [Tracking.mixin()],
...@@ -49,33 +49,27 @@ export default { ...@@ -49,33 +49,27 @@ export default {
</script> </script>
<template> <template>
<div <div class="gl-mr-3 gl-display-inline-block gl-vertical-align-bottom full-width-mobile">
data-testid="sort-discussion-filter"
class="gl-mr-2 gl-display-inline-block gl-vertical-align-bottom full-width-mobile"
>
<local-storage-sync <local-storage-sync
:value="sortDirection" :value="sortDirection"
:storage-key="storageKey" :storage-key="storageKey"
@input="setDiscussionSortDirection" @input="setDiscussionSortDirection"
/> />
<button class="btn btn-sm js-dropdown-text" data-toggle="dropdown" aria-expanded="false"> <gl-dropdown
{{ dropdownText }} :text="dropdownText"
<gl-icon name="chevron-down" /> data-testid="sort-discussion-filter"
</button> class="js-dropdown-text full-width-mobile"
<div ref="dropdownMenu" class="dropdown-menu dropdown-menu-selectable dropdown-menu-right"> >
<div class="dropdown-content"> <gl-dropdown-item
<ul> v-for="{ text, key, cls } in $options.SORT_OPTIONS"
<li v-for="{ text, key, cls } in $options.SORT_OPTIONS" :key="key"> :key="key"
<button :class="cls"
:class="[cls, { 'is-active': isDropdownItemActive(key) }]" :is-check-item="true"
type="button" :is-checked="isDropdownItemActive(key)"
@click="fetchSortedDiscussions(key)" @click="fetchSortedDiscussions(key)"
> >
{{ text }} {{ text }}
</button> </gl-dropdown-item>
</li> </gl-dropdown>
</ul>
</div>
</div>
</div> </div>
</template> </template>
...@@ -133,7 +133,7 @@ RSpec.describe 'Epic show', :js do ...@@ -133,7 +133,7 @@ RSpec.describe 'Epic show', :js do
it 'shows epic thread filter dropdown' do it 'shows epic thread filter dropdown' do
page.within('.js-noteable-awards') do page.within('.js-noteable-awards') do
expect(find('.js-discussion-filter-container #discussion-filter-dropdown')).to have_content('Show all activity') expect(find('#discussion-filter-dropdown')).to have_content('Show all activity')
end end
end end
...@@ -142,9 +142,7 @@ RSpec.describe 'Epic show', :js do ...@@ -142,9 +142,7 @@ RSpec.describe 'Epic show', :js do
context 'when sorted by `Oldest first`' do context 'when sorted by `Oldest first`' do
it 'shows comments in the correct order' do it 'shows comments in the correct order' do
page.within('[data-testid="sort-discussion-filter"]') do
expect(find('.js-dropdown-text')).to have_content('Oldest first') expect(find('.js-dropdown-text')).to have_content('Oldest first')
end
items = all('.timeline-entry .timeline-discussion-body .note-text') items = all('.timeline-entry .timeline-discussion-body .note-text')
expect(items[0]).to have_content(notes[0].note) expect(items[0]).to have_content(notes[0].note)
...@@ -155,7 +153,7 @@ RSpec.describe 'Epic show', :js do ...@@ -155,7 +153,7 @@ RSpec.describe 'Epic show', :js do
context 'when sorted by `Newest first`' do context 'when sorted by `Newest first`' do
before do before do
page.within('[data-testid="sort-discussion-filter"]') do page.within('[data-testid="sort-discussion-filter"]') do
find('button').click find('.js-dropdown-text').click
find('.js-newest-first').click find('.js-newest-first').click
wait_for_requests wait_for_requests
end end
...@@ -163,7 +161,7 @@ RSpec.describe 'Epic show', :js do ...@@ -163,7 +161,7 @@ RSpec.describe 'Epic show', :js do
it 'shows comments in the correct order', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/225637' do it 'shows comments in the correct order', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/225637' do
page.within('[data-testid="sort-discussion-filter"]') do page.within('[data-testid="sort-discussion-filter"]') do
expect(find('.js-dropdown-text')).to have_content('Newest first') expect(find('.js-newest-first')).to have_content('Newest first')
end end
items = all('.timeline-entry .timeline-discussion-body .note-text') items = all('.timeline-entry .timeline-discussion-body .note-text')
......
...@@ -74,13 +74,15 @@ describe('DiscussionFilter component', () => { ...@@ -74,13 +74,15 @@ describe('DiscussionFilter component', () => {
}); });
it('renders the all filters', () => { it('renders the all filters', () => {
expect(wrapper.findAll('.dropdown-menu li').length).toBe(discussionFiltersMock.length); expect(wrapper.findAll('.discussion-filter-container .dropdown-item').length).toBe(
discussionFiltersMock.length,
);
}); });
it('renders the default selected item', () => { it('renders the default selected item', () => {
expect( expect(
wrapper wrapper
.find('#discussion-filter-dropdown') .find('#discussion-filter-dropdown .dropdown-item')
.text() .text()
.trim(), .trim(),
).toBe(discussionFiltersMock[0].title); ).toBe(discussionFiltersMock[0].title);
...@@ -88,7 +90,7 @@ describe('DiscussionFilter component', () => { ...@@ -88,7 +90,7 @@ describe('DiscussionFilter component', () => {
it('updates to the selected item', () => { it('updates to the selected item', () => {
const filterItem = wrapper.find( const filterItem = wrapper.find(
`.dropdown-menu li[data-filter-type="${DISCUSSION_FILTER_TYPES.HISTORY}"] button`, `.discussion-filter-container .dropdown-item[data-filter-type="${DISCUSSION_FILTER_TYPES.HISTORY}"]`,
); );
filterItem.trigger('click'); filterItem.trigger('click');
...@@ -98,7 +100,9 @@ describe('DiscussionFilter component', () => { ...@@ -98,7 +100,9 @@ describe('DiscussionFilter component', () => {
it('only updates when selected filter changes', () => { it('only updates when selected filter changes', () => {
wrapper wrapper
.find(`.dropdown-menu li[data-filter-type="${DISCUSSION_FILTER_TYPES.ALL}"] button`) .find(
`.discussion-filter-container .dropdown-item[data-filter-type="${DISCUSSION_FILTER_TYPES.ALL}"]`,
)
.trigger('click'); .trigger('click');
expect(filterDiscussion).not.toHaveBeenCalled(); expect(filterDiscussion).not.toHaveBeenCalled();
...@@ -106,7 +110,7 @@ describe('DiscussionFilter component', () => { ...@@ -106,7 +110,7 @@ describe('DiscussionFilter component', () => {
it('disables commenting when "Show history only" filter is applied', () => { it('disables commenting when "Show history only" filter is applied', () => {
const filterItem = wrapper.find( const filterItem = wrapper.find(
`.dropdown-menu li[data-filter-type="${DISCUSSION_FILTER_TYPES.HISTORY}"] button`, `.discussion-filter-container .dropdown-item[data-filter-type="${DISCUSSION_FILTER_TYPES.HISTORY}"]`,
); );
filterItem.trigger('click'); filterItem.trigger('click');
...@@ -115,7 +119,7 @@ describe('DiscussionFilter component', () => { ...@@ -115,7 +119,7 @@ describe('DiscussionFilter component', () => {
it('enables commenting when "Show history only" filter is not applied', () => { it('enables commenting when "Show history only" filter is not applied', () => {
const filterItem = wrapper.find( const filterItem = wrapper.find(
`.dropdown-menu li[data-filter-type="${DISCUSSION_FILTER_TYPES.ALL}"] button`, `.discussion-filter-container .dropdown-item[data-filter-type="${DISCUSSION_FILTER_TYPES.ALL}"]`,
); );
filterItem.trigger('click'); filterItem.trigger('click');
...@@ -124,10 +128,10 @@ describe('DiscussionFilter component', () => { ...@@ -124,10 +128,10 @@ describe('DiscussionFilter component', () => {
it('renders a dropdown divider for the default filter', () => { it('renders a dropdown divider for the default filter', () => {
const defaultFilter = wrapper.findAll( const defaultFilter = wrapper.findAll(
`.dropdown-menu li[data-filter-type="${DISCUSSION_FILTER_TYPES.ALL}"] > *`, `.discussion-filter-container .dropdown-item-wrapper > *`,
); );
expect(defaultFilter.at(defaultFilter.length - 1).classes('dropdown-divider')).toBe(true); expect(defaultFilter.at(1).classes('gl-new-dropdown-divider')).toBe(true);
}); });
describe('Merge request tabs', () => { describe('Merge request tabs', () => {
......
...@@ -55,7 +55,7 @@ describe('Sort Discussion component', () => { ...@@ -55,7 +55,7 @@ describe('Sort Discussion component', () => {
it('calls the right actions', () => { it('calls the right actions', () => {
createComponent(); createComponent();
wrapper.find('.js-newest-first').trigger('click'); wrapper.find('.js-newest-first').vm.$emit('click');
expect(store.dispatch).toHaveBeenCalledWith('setDiscussionSortDirection', DESC); expect(store.dispatch).toHaveBeenCalledWith('setDiscussionSortDirection', DESC);
expect(Tracking.event).toHaveBeenCalledWith(undefined, 'change_discussion_sort_direction', { expect(Tracking.event).toHaveBeenCalledWith(undefined, 'change_discussion_sort_direction', {
...@@ -67,7 +67,7 @@ describe('Sort Discussion component', () => { ...@@ -67,7 +67,7 @@ describe('Sort Discussion component', () => {
it('shows the "Oldest First" as the dropdown', () => { it('shows the "Oldest First" as the dropdown', () => {
createComponent(); createComponent();
expect(wrapper.find('.js-dropdown-text').text()).toBe('Oldest first'); expect(wrapper.find('.js-dropdown-text').props('text')).toBe('Oldest first');
}); });
}); });
...@@ -79,7 +79,7 @@ describe('Sort Discussion component', () => { ...@@ -79,7 +79,7 @@ describe('Sort Discussion component', () => {
describe('when the dropdown item is clicked', () => { describe('when the dropdown item is clicked', () => {
it('calls the right actions', () => { it('calls the right actions', () => {
wrapper.find('.js-oldest-first').trigger('click'); wrapper.find('.js-oldest-first').vm.$emit('click');
expect(store.dispatch).toHaveBeenCalledWith('setDiscussionSortDirection', ASC); expect(store.dispatch).toHaveBeenCalledWith('setDiscussionSortDirection', ASC);
expect(Tracking.event).toHaveBeenCalledWith(undefined, 'change_discussion_sort_direction', { expect(Tracking.event).toHaveBeenCalledWith(undefined, 'change_discussion_sort_direction', {
...@@ -87,13 +87,13 @@ describe('Sort Discussion component', () => { ...@@ -87,13 +87,13 @@ describe('Sort Discussion component', () => {
}); });
}); });
it('applies the active class to the correct button in the dropdown', () => { it('sets is-checked to true on the active button in the dropdown', () => {
expect(wrapper.find('.js-newest-first').classes()).toContain('is-active'); expect(wrapper.find('.js-newest-first').props('isChecked')).toBe(true);
}); });
}); });
it('shows the "Newest First" as the dropdown', () => { it('shows the "Newest First" as the dropdown', () => {
expect(wrapper.find('.js-dropdown-text').text()).toBe('Newest first'); expect(wrapper.find('.js-dropdown-text').props('text')).toBe('Newest first');
}); });
}); });
}); });
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