Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(plugin): do not show drag group sort when column is not sortable #819

Merged
merged 1 commit into from
Nov 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { BackendUtilityService, createDomElement, } from '../../services';
import { SharedService } from '../../services/shared.service';
import { TranslateServiceStub } from '../../../../../test/translateServiceStub';
import { SortDirectionNumber } from '../../enums';
import { deepCopy } from '@slickgrid-universal/utils';

declare const Slick: SlickNamespace;
const GRID_UID = 'slickgrid12345';
Expand Down Expand Up @@ -93,15 +94,15 @@ const mockColumns = [
{ id: 'firstName', name: 'First Name', field: 'firstName', width: 100 },
{ id: 'lastName', name: 'Last Name', field: 'lastName', width: 100 },
{
id: 'age', name: 'Age', field: 'age', width: 50,
id: 'age', name: 'Age', field: 'age', width: 50, sortable: true,
grouping: {
getter: 'age', aggregators: [new Aggregators.Avg('age')],
formatter: (g) => `Age: ${g.value} <span style="color:green">(${g.count} items)</span>`,
collapsed: true
}
},
{
id: 'medals', name: 'Medals', field: 'medals', width: 50,
id: 'medals', name: 'Medals', field: 'medals', width: 50,sortable: true,
grouping: {
getter: 'medals', aggregators: [new Aggregators.Sum('medals')],
formatter: (g) => `Medals: ${g.value} <span style="color:green">(${g.count} items)</span>`,
Expand Down Expand Up @@ -583,6 +584,26 @@ describe('Draggable Grouping Plugin', () => {
expect(groupBySortAscIconElm).toBeFalsy();
});

it('should not expect any sort icons displayed when the Column is not Sortable', () => {
const mockColumnsCopy = deepCopy(mockColumns);
mockColumnsCopy[2].sortable = false; // change age column to not sortable
jest.spyOn(gridStub, 'getColumns').mockReturnValueOnce(mockColumnsCopy);
plugin.init(gridStub, { ...addonOptions });
const fn = plugin.setupColumnReorder(gridStub, mockHeaderLeftDiv1, {}, setColumnsSpy, setColumnResizeSpy, mockColumnsCopy, getColumnIndexSpy, GRID_UID, triggerSpy);
jest.spyOn(fn.sortableLeftInstance, 'toArray').mockReturnValue(['age']);

fn.sortableLeftInstance!.options.onStart!({} as any);
plugin.droppableInstance!.options.onAdd!({ item: headerColumnDiv3, clone: headerColumnDiv3.cloneNode(true) } as any);

let groupBySortElm = preHeaderDiv.querySelector('.slick-groupby-sort') as HTMLDivElement;
let groupBySortAscIconElm = preHeaderDiv.querySelector('.slick-groupby-sort-asc-icon') as HTMLDivElement;

// we're not hiding the columns, but it's not Sortable so the result is the same
expect(plugin.addonOptions.hideGroupSortIcons).toBe(false);
expect(groupBySortElm).toBeFalsy();
expect(groupBySortAscIconElm).toBeFalsy();
});

it('should toggle ascending/descending order when original sort is ascending then user clicked the sorting icon twice', () => {
plugin.init(gridStub, { ...addonOptions });
const fn = plugin.setupColumnReorder(gridStub, mockHeaderLeftDiv1, {}, setColumnsSpy, setColumnResizeSpy, mockColumns, getColumnIndexSpy, GRID_UID, triggerSpy);
Expand Down
5 changes: 3 additions & 2 deletions packages/common/src/extensions/slickDraggableGrouping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -468,9 +468,10 @@ export class SlickDraggableGrouping {
groupRemoveIconElm.classList.add('slick-groupby-remove-icon');
}

// sorting icons
// sorting icons when enabled
let groupSortContainerElm: HTMLDivElement | undefined;
if (this._addonOptions?.hideGroupSortIcons !== true) {
console.log('col id:', col.id, 'sortable:', col.sortable);
if (this._addonOptions?.hideGroupSortIcons !== true && col.sortable) {
if (col.grouping?.sortAsc === undefined) {
col.grouping.sortAsc = true;
}
Expand Down