From 7f4fcda1d264bf5feead51adb2d11823b52d0726 Mon Sep 17 00:00:00 2001 From: Petra Laohakul Date: Tue, 19 Apr 2022 13:12:53 -0400 Subject: [PATCH 1/4] Checkpoint - fix visibility issues when grouping and ungrouping --- cmp/grid/Grid.js | 1 + cmp/grid/GridModel.js | 16 ++++++++++++++-- cmp/grid/columns/Column.js | 17 +++++++++++++++-- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/cmp/grid/Grid.js b/cmp/grid/Grid.js index 08a0234ce5..436a993dab 100644 --- a/cmp/grid/Grid.js +++ b/cmp/grid/Grid.js @@ -232,6 +232,7 @@ class GridLocalModel extends HoistModel { navigateToNextCell: this.navigateToNextCell, processCellForClipboard: this.processCellForClipboard, defaultGroupOrderComparator: model.groupSortFn ? this.groupSortComparator : undefined, + suppressMakeColumnVisibleAfterUnGroup: true, groupDefaultExpanded: 1, groupDisplayType: 'groupRows', groupRowRenderer: model.groupRowElementRenderer, diff --git a/cmp/grid/GridModel.js b/cmp/grid/GridModel.js index 34f01363fe..0ba9d28ae6 100644 --- a/cmp/grid/GridModel.js +++ b/cmp/grid/GridModel.js @@ -31,7 +31,7 @@ import { cloneDeep, compact, defaults, - defaultsDeep, + defaultsDeep, difference, find, forEach, isArray, @@ -708,13 +708,25 @@ export class GridModel extends HoistModel { @action setGroupBy(colIds) { colIds = isNil(colIds) ? [] : castArray(colIds); + const getCol = (colId) => this.findColumn(this.columns, colId), + invalidColIds = colIds.filter(it => !getCol(it)); - const invalidColIds = colIds.filter(it => !this.findColumn(this.columns, it)); if (invalidColIds.length) { console.warn('Unknown colId specified in groupBy - grid will not be grouped.', invalidColIds); colIds = []; } + const {groupBy} = this, + ungroupedColIds = difference(groupBy, colIds); + + colIds.forEach(it => { + if (getCol(it).hideWhenGrouped) this.hideColumn(it); + }); + + ungroupedColIds.forEach(it => { + if (getCol(it)?.showWhenUngrouped) this.showColumn(it); + }); + this.groupBy = colIds; } diff --git a/cmp/grid/columns/Column.js b/cmp/grid/columns/Column.js index bf3ad7086f..57ba1226d1 100644 --- a/cmp/grid/columns/Column.js +++ b/cmp/grid/columns/Column.js @@ -94,7 +94,11 @@ export class Column { align; /** @member {boolean} */ - hidden + hidden; + /** @member {boolean} */ + hideWhenGrouped; + /** @member {boolean} */ + showWhenUngrouped; /** @member {(boolean|number)} */ flex; /** @member {number} */ @@ -225,6 +229,8 @@ export class Column { * class names to functions determining if they should be added or removed from the cell. * See Ag-Grid docs on "cell styles" for details. * @param {boolean} [c.hidden] - true to suppress default display of the column. + * @param {boolean} [c.hideWhenGrouped] - true to hide column when grouped. + * @param {boolean} [c.showWhenUngrouped] - true to show column when ungrouped. * @param {string} [c.align] - horizontal alignment of cell contents. * Valid values are: 'left' (default), 'right' or 'center'. * @param {number} [c.width] - default width in pixels. @@ -338,6 +344,8 @@ export class Column { cellClass, cellClassRules, hidden, + hideWhenGrouped, + showWhenUngrouped, align, width, minWidth, @@ -417,6 +425,8 @@ export class Column { this.align = align; this.hidden = withDefault(hidden, false); + this.hideWhenGrouped = withDefault(hideWhenGrouped, true); + this.showWhenUngrouped = withDefault(showWhenUngrouped, false); warnIf(rest.hide, `Column ${this.colId} configured with {hide: true} - use "hidden" instead.`); warnIf( @@ -467,7 +477,10 @@ export class Column { this.chooserName = chooserName || this.displayName; this.chooserGroup = chooserGroup; this.chooserDescription = chooserDescription; - this.excludeFromChooser = withDefault(excludeFromChooser, false); + this.excludeFromChooser = withDefault(showWhenUngrouped && hidden ? false : excludeFromChooser, false); + warnIf(showWhenUngrouped && hidden && excludeFromChooser, + `Column "${this.colId}" cannot be configured with {hidden: true, excludeFromChooser: true} AND {showWhenUngrouped: true}. ExcludeFromChooser will be ignored.` + ); // ExportName must be non-empty string. Default to headerName if unspecified (it supports // the function form of headerName) and fallback to colId. Note GridExportService can From 2941df63d2d82cd565ff9984da62bf712c41a3dd Mon Sep 17 00:00:00 2001 From: Petra Laohakul Date: Tue, 19 Apr 2022 19:08:31 -0400 Subject: [PATCH 2/4] Move config to set visibility on grouped columns to GridModel. + Changelog update --- CHANGELOG.md | 7 ++++++- cmp/grid/GridModel.js | 24 +++++++++++++++++++++--- cmp/grid/columns/Column.js | 31 ++++++++++++++----------------- 3 files changed, 41 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 616c19b032..2f58fdead3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ * `fmtQuantity` now displays values greater than one billion with `b` unit, similar to current handling of millions with `m`. * Enhancements to admin log viewer to show timezone and log file metadata. +* Added `GridModel.hideGroupedColumns` and `GridModel.showUngroupedColumns` to manage visibility of + columns when grouped and ungrouped. Both default to true. ### 💥 Breaking Changes @@ -27,9 +29,12 @@ ### 🐞 Bug Fixes -* Set ag-Grid's `suppressLastEmptyLineOnPaste` to true to work around a bug with Excel (Windows) +* Set ag-Grid's `suppressLastEmptyLineOnPaste` to `true` to work around a bug with Excel (Windows) that adds an empty line beneath the range pasted from the clipboard in editable grids. * Fixes an issue where NumberInput would initially render blank values when `max` and `min` were set. +* Set ag-Grid's `suppressMakeColumnVisibleAfterUnGroup` to `true` to avoid unwanted visibility state + changes when `Column.excludeFromChooser` is set to `true`. New `GridModel.hideGroupedColumns` and + `GridModel.showUngroupedColumns` will not affect columns that have their visibility or hidden states locked. ### 📚 Libraries diff --git a/cmp/grid/GridModel.js b/cmp/grid/GridModel.js index 0ba9d28ae6..1b2c8c891c 100644 --- a/cmp/grid/GridModel.js +++ b/cmp/grid/GridModel.js @@ -102,6 +102,10 @@ export class GridModel extends HoistModel { /** @member {boolean} */ showGroupRowCounts; /** @member {boolean} */ + hideGroupedColumns; + /** @member {boolean} */ + showUngroupedColumns; + /** @member {boolean} */ enableColumnPinning; /** @member {boolean} */ enableExport; @@ -267,6 +271,8 @@ export class GridModel extends HoistModel { * @param {GridGroupSortFn} [c.groupSortFn] - function to use to sort full-row groups. * Called with two group values to compare in the form of a standard JS comparator. * Default is an ascending string sort. Set to `null` to prevent sorting of groups. + * @param {boolean} [c.hideGroupedColumns] - true to hide grouped columns. Default true. + * @param {boolean} [c.showUngroupedColumns] - true to show ungrouped columns. Default true. * @param {function} [c.onKeyDown] - Callback when a key down event is detected on the * grid. Function will receive an event with the standard 'target' element. Note that * the ag-Grid API provides limited ability to customize keyboard handling. This handler @@ -352,6 +358,8 @@ export class GridModel extends HoistModel { groupRowRenderer, groupRowElementRenderer, groupSortFn, + hideGroupedColumns = true, + showUngroupedColumns = true, onKeyDown, onRowClicked, @@ -389,6 +397,8 @@ export class GridModel extends HoistModel { this.groupRowElementRenderer = groupRowElementRenderer; this.groupSortFn = withDefault(groupSortFn, this.defaultGroupSortFn); this.showGroupRowCounts = showGroupRowCounts; + this.hideGroupedColumns = hideGroupedColumns; + this.showUngroupedColumns = showUngroupedColumns; this.contextMenu = withDefault(contextMenu, GridModel.defaultContextMenu); this.useVirtualColumns = useVirtualColumns; this.externalSort = externalSort; @@ -716,15 +726,23 @@ export class GridModel extends HoistModel { colIds = []; } - const {groupBy} = this, + // Set column visibility as specified, respecting locked visible/hidden states. + const {groupBy, hideGroupedColumns, showUngroupedColumns, colChooserModel} = this, ungroupedColIds = difference(groupBy, colIds); colIds.forEach(it => { - if (getCol(it).hideWhenGrouped) this.hideColumn(it); + if (getCol(it).lockVisible) return; + if (hideGroupedColumns) this.hideColumn(it); }); ungroupedColIds.forEach(it => { - if (getCol(it)?.showWhenUngrouped) this.showColumn(it); + const col = getCol(it); + if (col.lockHidden) return; + if (showUngroupedColumns) this.showColumn(it); + + if (!showUngroupedColumns && hideGroupedColumns && (col.excludeFromChooser || !colChooserModel)) { + console.warn('Column cannot be made visible after grouping due to GridModel and/or Column configuration. colId:', col.colId); + } }); this.groupBy = colIds; diff --git a/cmp/grid/columns/Column.js b/cmp/grid/columns/Column.js index 57ba1226d1..56704eb27f 100644 --- a/cmp/grid/columns/Column.js +++ b/cmp/grid/columns/Column.js @@ -95,10 +95,6 @@ export class Column { /** @member {boolean} */ hidden; - /** @member {boolean} */ - hideWhenGrouped; - /** @member {boolean} */ - showWhenUngrouped; /** @member {(boolean|number)} */ flex; /** @member {number} */ @@ -229,8 +225,6 @@ export class Column { * class names to functions determining if they should be added or removed from the cell. * See Ag-Grid docs on "cell styles" for details. * @param {boolean} [c.hidden] - true to suppress default display of the column. - * @param {boolean} [c.hideWhenGrouped] - true to hide column when grouped. - * @param {boolean} [c.showWhenUngrouped] - true to show column when ungrouped. * @param {string} [c.align] - horizontal alignment of cell contents. * Valid values are: 'left' (default), 'right' or 'center'. * @param {number} [c.width] - default width in pixels. @@ -281,8 +275,8 @@ export class Column { * @param {string} [c.chooserName] - name to display within the column chooser component. * Defaults to `displayName`, can be longer / less abbreviated than `headerName` might be. * @param {string} [c.chooserGroup] - group name to display within the column chooser - * component. - * Chooser will automatically group its "available columns" grid if any cols provide. + * component. Chooser will automatically group its "available columns" grid if any cols + * provide. * @param {string} [c.chooserDescription] - additional descriptive text to display within the * column chooser. Appears when the column is selected within the chooser UI. * @param {boolean} [c.excludeFromChooser] - true to hide the column from the column chooser @@ -344,8 +338,6 @@ export class Column { cellClass, cellClassRules, hidden, - hideWhenGrouped, - showWhenUngrouped, align, width, minWidth, @@ -425,8 +417,6 @@ export class Column { this.align = align; this.hidden = withDefault(hidden, false); - this.hideWhenGrouped = withDefault(hideWhenGrouped, true); - this.showWhenUngrouped = withDefault(showWhenUngrouped, false); warnIf(rest.hide, `Column ${this.colId} configured with {hide: true} - use "hidden" instead.`); warnIf( @@ -477,10 +467,7 @@ export class Column { this.chooserName = chooserName || this.displayName; this.chooserGroup = chooserGroup; this.chooserDescription = chooserDescription; - this.excludeFromChooser = withDefault(showWhenUngrouped && hidden ? false : excludeFromChooser, false); - warnIf(showWhenUngrouped && hidden && excludeFromChooser, - `Column "${this.colId}" cannot be configured with {hidden: true, excludeFromChooser: true} AND {showWhenUngrouped: true}. ExcludeFromChooser will be ignored.` - ); + this.excludeFromChooser = withDefault(excludeFromChooser, false); // ExportName must be non-empty string. Default to headerName if unspecified (it supports // the function form of headerName) and fallback to colId. Note GridExportService can @@ -517,6 +504,16 @@ export class Column { warnIf(this.agOptions.valueGetter, `Column '${this.colId}' uses valueGetter through agOptions. Remove and use custom getValueFn if needed.`); } + /** @return {boolean} - true if column should always remain visible */ + get lockVisible() { + return !this.hideable || !this.gridModel.colChooserModel || XH.isMobileApp; + } + + /** @return {boolean} - true if column is initialized as hidden and cannot be made visible via ChooserModel. */ + get lockHidden() { + return this.hidden && (this.excludeFromChooser || !this.gridModel.colChooserModel); + } + /** * @param {StoreRecord} record * @return {boolean} - true if this column supports editing its field for the given StoreRecord. @@ -553,7 +550,7 @@ export class Column { suppressMovable: !this.movable, lockPinned: !gridModel.enableColumnPinning || XH.isMobileApp, pinned: this.pinned, - lockVisible: !this.hideable || !gridModel.colChooserModel || XH.isMobileApp, + lockVisible: this.lockVisible, headerComponentParams: {gridModel, xhColumn: this}, suppressColumnsToolPanel: this.excludeFromChooser, suppressFiltersToolPanel: this.excludeFromChooser, From a34dce9b922e1650cb7fefe45f3c470b615e5a16 Mon Sep 17 00:00:00 2001 From: Petra Laohakul Date: Tue, 19 Apr 2022 19:13:01 -0400 Subject: [PATCH 3/4] Fix import formatting --- cmp/grid/GridModel.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmp/grid/GridModel.js b/cmp/grid/GridModel.js index 1b2c8c891c..3754daa929 100644 --- a/cmp/grid/GridModel.js +++ b/cmp/grid/GridModel.js @@ -31,7 +31,8 @@ import { cloneDeep, compact, defaults, - defaultsDeep, difference, + defaultsDeep, + difference, find, forEach, isArray, From f26b796c26302feae60cf9d1767ea476c2b3df1a Mon Sep 17 00:00:00 2001 From: Petra Laohakul Date: Tue, 19 Apr 2022 19:19:15 -0400 Subject: [PATCH 4/4] Tweak to lockVisible getter --- cmp/grid/columns/Column.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmp/grid/columns/Column.js b/cmp/grid/columns/Column.js index 56704eb27f..ab415b8952 100644 --- a/cmp/grid/columns/Column.js +++ b/cmp/grid/columns/Column.js @@ -506,7 +506,7 @@ export class Column { /** @return {boolean} - true if column should always remain visible */ get lockVisible() { - return !this.hideable || !this.gridModel.colChooserModel || XH.isMobileApp; + return !this.hideable || !this.gridModel.colChooserModel; } /** @return {boolean} - true if column is initialized as hidden and cannot be made visible via ChooserModel. */ @@ -550,7 +550,7 @@ export class Column { suppressMovable: !this.movable, lockPinned: !gridModel.enableColumnPinning || XH.isMobileApp, pinned: this.pinned, - lockVisible: this.lockVisible, + lockVisible: this.lockVisible || XH.isMobileApp, headerComponentParams: {gridModel, xhColumn: this}, suppressColumnsToolPanel: this.excludeFromChooser, suppressFiltersToolPanel: this.excludeFromChooser,