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.

Sign up for GitHub

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: hideColumnByIds wasn't hiding columns properly #1738

Merged
merged 2 commits into from
Nov 9, 2024
Merged

fix: hideColumnByIds wasn't hiding columns properly #1738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d46daf
fix: hideColumnByIds wasn't hiding columns properly
ghiscoding Nov 9, 2024
e32d9f0
refactor: use better function name
ghiscoding Nov 9, 2024
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
12 changes: 8 additions & 4 deletions packages/common/src/extensions/__tests__/slickGridMenu.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('GridMenuControl', () => {
const columnsMock: Column[] = [
{ id: 'field1', field: 'field1', name: 'Field 1', width: 100, nameKey: 'TITLE' },
{ id: 'field2', field: 'field2', name: 'Field 2', width: 75 },
{ id: 'field3', field: 'field3', name: 'Field 3', width: 75, columnGroup: 'Billing' },
{ id: 'field3', field: 'field3', name: 'Field 3', width: 75, columnGroup: 'Billing', excludeFromGridMenu: true },
];
let backendUtilityService: BackendUtilityService;
let extensionUtility: ExtensionUtility;
Expand Down Expand Up @@ -681,7 +681,11 @@ describe('GridMenuControl', () => {
const forceFitElm = control.menuElement!.querySelector('#slickgrid_124343-gridmenu-colpicker-forcefit') as HTMLInputElement;
const inputSyncElm = control.menuElement!.querySelector('#slickgrid_124343-gridmenu-colpicker-syncresize') as HTMLInputElement;
const pickerField1Elm = document.querySelector('input[type="checkbox"][data-columnid="field1"]') as HTMLInputElement;
const li2Elm = document.querySelector('.slick-column-picker-list li:nth-of-type(2)') as HTMLLIElement;
const li3Elm = document.querySelector('.slick-column-picker-list li:nth-of-type(3)') as HTMLLIElement;
expect(pickerField1Elm.checked).toBe(true);
expect(li2Elm.className).not.toBe('hidden');
expect(li3Elm.className).toBe('hidden');
pickerField1Elm.checked = false;
pickerField1Elm.dispatchEvent(new Event('click', { bubbles: true, cancelable: true, composed: false }));

Expand Down Expand Up @@ -1559,12 +1563,12 @@ describe('GridMenuControl', () => {
const columnsUnorderedMock: Column[] = [
{ id: 'field2', field: 'field2', name: 'Field 2', width: 75 },
{ id: 'field1', field: 'field1', name: 'Titre', width: 100, nameKey: 'TITLE' },
{ id: 'field3', field: 'field3', name: 'Field 3', width: 75, columnGroup: 'Billing' },
{ id: 'field3', field: 'field3', name: 'Field 3', width: 75, columnGroup: 'Billing', excludeFromGridMenu: true },
];
const columnsMock: Column[] = [
{ id: 'field1', field: 'field1', name: 'Titre', width: 100, nameKey: 'TITLE' },
{ id: 'field2', field: 'field2', name: 'Field 2', width: 75 },
{ id: 'field3', field: 'field3', name: 'Field 3', width: 75, columnGroup: 'Billing' },
{ id: 'field3', field: 'field3', name: 'Field 3', width: 75, columnGroup: 'Billing', excludeFromGridMenu: true },
];
vi.spyOn(gridStub, 'getColumnIndex').mockReturnValue(undefined as any).mockReturnValueOnce(0).mockReturnValueOnce(1);
const handlerSpy = vi.spyOn(control.eventHandler, 'subscribe');
Expand Down Expand Up @@ -1630,7 +1634,7 @@ describe('GridMenuControl', () => {
expect(columnsMock).toEqual([
{ id: 'field1', field: 'field1', name: 'Titre', width: 100, nameKey: 'TITLE' },
{ id: 'field2', field: 'field2', name: 'Field 2', width: 75 },
{ id: 'field3', field: 'field3', name: 'Field 3', columnGroup: 'Billing', width: 75 },
{ id: 'field3', field: 'field3', name: 'Field 3', columnGroup: 'Billing', width: 75, excludeFromGridMenu: true },
]);
expect(control.getAllColumns()).toEqual(columnsMock);
expect(control.getVisibleColumns()).toEqual(columnsMock);
Expand Down
5 changes: 3 additions & 2 deletions packages/common/src/extensions/extensionCommonUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,13 @@ function generatePickerCheckbox(columnLiElm: HTMLLIElement, inputId: string, inp

export function populateColumnPicker(this: SlickColumnPicker | SlickGridMenu, addonOptions: ColumnPickerOption | GridMenuOption): void {
const context: any = this;
const menuPrefix = context instanceof SlickGridMenu ? 'gridmenu-' : '';
const isGridMenu = context instanceof SlickGridMenu;
const menuPrefix = isGridMenu ? 'gridmenu-' : '';

for (const column of context.columns) {
const columnId = column.id;
const columnLiElm = document.createElement('li');
if (column.excludeFromColumnPicker) {
if ((column.excludeFromColumnPicker && !isGridMenu) || (column.excludeFromGridMenu && isGridMenu)) {
columnLiElm.className = 'hidden';
}

Expand Down
54 changes: 23 additions & 31 deletions packages/common/src/services/grid.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,10 @@ export class GridService {

if (colIndexFound >= 0) {
const visibleColumns = arrayRemoveItemByIndex<Column>(currentColumns, colIndexFound);
this.sharedService.visibleColumns = visibleColumns;

// do we want to apply the new columns in the grid
if (options?.applySetColumns) {
this.sharedService.visibleColumns = visibleColumns;
this._grid.setColumns(visibleColumns);
}

Expand All @@ -222,15 +222,8 @@ export class GridService {
}
}

// do we want to auto-resize the columns in the grid after hidding some? most often yes
if (options?.autoResizeColumns) {
this._grid.autosizeColumns();
}

// do we want to trigger an event after hidding
if (options?.triggerEvent) {
this.pubSubService.publish('onHeaderMenuHideColumns', { columns: visibleColumns });
}
// execute common grid commands when enabled
this.executeVisibilityCommands(options, ['onHeaderMenuHideColumns'], visibleColumns);
return colIndexFound;
}
}
Expand All @@ -244,26 +237,20 @@ export class GridService {
*/
hideColumnByIds(columnIds: Array<string | number>, options?: HideColumnOption): void {
if (Array.isArray(columnIds)) {
const finalVisibileColumns = this._grid.getColumns().filter(c => !columnIds.includes(c.id));
options = { ...HideColumnOptionDefaults, ...options };
for (const columnId of columnIds) {
// hide each column by its id but wait after the for loop to auto resize columns in the grid
this.hideColumnById(columnId, { ...options, triggerEvent: false, applySetColumns: false, autoResizeColumns: false });
}

// after looping through all columns, we can apply the new columns in the grid
this._grid.setColumns(this.sharedService.visibleColumns);
// after looping through all columns, we can apply the leftover visible columns in the grid & keep shared ref
this.sharedService.visibleColumns = finalVisibileColumns;
this._grid.setColumns(finalVisibileColumns);

// do we want to auto-resize the columns in the grid after hidding some? most often yes
if (options?.autoResizeColumns) {
this._grid.autosizeColumns();
}

// do we want to trigger an event after hidding
if (options?.triggerEvent) {
// @deprecate `onHeaderMenuHideColumns` event, we should keep only `onHideColumns`
this.pubSubService.publish('onHeaderMenuHideColumns', { columns: this.sharedService.visibleColumns });
this.pubSubService.publish('onHideColumns', { columns: this.sharedService.visibleColumns });
}
// execute common grid commands when enabled
// @deprecate `onHeaderMenuHideColumns` event, we should keep only `onHideColumns`
this.executeVisibilityCommands(options, ['onHeaderMenuHideColumns', 'onHideColumns'], finalVisibileColumns);
}
}

Expand All @@ -279,15 +266,20 @@ export class GridService {
this._grid.setColumns(columns);
this.sharedService.visibleColumns = columns;

// do we want to auto-resize the columns in the grid after hidding some? most often yes
if (options?.autoResizeColumns) {
this._grid.autosizeColumns();
}
// execute common grid commands when enabled
this.executeVisibilityCommands(options, ['onShowColumns'], this.sharedService.visibleColumns);
}
}

// do we want to trigger an event after showing
if (options?.triggerEvent) {
this.pubSubService.publish('onShowColumns', { columns: this.sharedService.visibleColumns });
}
protected executeVisibilityCommands(options: { autoResizeColumns?: boolean; triggerEvent?: boolean; }, eventNames: string[], columns: Column[]): void {
// do we want to auto-resize the columns in the grid after hidding/showing columns?
if (options?.autoResizeColumns) {
this._grid.autosizeColumns();
}

// do we want to trigger an event after showing
if (options?.triggerEvent) {
eventNames.forEach(name => this.pubSubService.publish(name, { columns }));
}
}

Expand Down
Loading