From 012dfeb885c1ceeccbc2afc3825c6755a2baaf1c Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Tue, 23 Jul 2024 21:11:40 +0100 Subject: [PATCH 01/13] set table select cell content as label + tests --- .../TableV2/columnTypes/Select1_spec.ts | 40 +++++++++++++++++-- .../component/cellComponents/SelectCell.tsx | 13 +++++- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts index ba126c6e70f..58dbfda12fe 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts @@ -174,7 +174,39 @@ describe( cy.discardTableRow(4, 0); }); - it("6. should check that currentRow is accessible in the select options", () => { + it("6. should check that on option select uses label as cell content in select cell", () => { + cy.updateCodeInput( + ".t--property-control-options", + ` + [ + { + "label": "#1label", + "value": "#1value" + }, + { + "label": "#2label", + "value": "#2value" + }, + { + "label": "#3label", + "value": "#3value" + } + ] + `, + ); + cy.editTableSelectCell(0, 0); + cy.get(".menu-item-link").contains("#3label").click(); + + _.agHelper.ValidateToastMessage("#3value"); + + cy.get(".menu-virtual-list").should("not.exist"); + cy.readTableV2data(0, 0).then((val) => { + expect(val).to.equal("#3label"); + }); + cy.discardTableRow(4, 0); + }); + + it("7. should check that currentRow is accessible in the select options", () => { cy.updateCodeInput( ".t--property-control-options", ` @@ -199,7 +231,7 @@ describe( cy.get(".menu-item-text").contains("#1").should("not.exist"); }); - it("7. should check that 'same select option in new row' property is working", () => { + it("8. should check that 'same select option in new row' property is working", () => { _.propPane.NavigateBackToPropertyPane(); const checkSameOptionsInNewRowWhileEditing = () => { @@ -265,7 +297,7 @@ describe( checkSameOptionsWhileAddingNewRow(); }); - it("8. should check that 'new row select options' is working", () => { + it("9. should check that 'new row select options' is working", () => { const checkNewRowOptions = () => { // New row select options should be visible when "Same options in new row" is turned off _.propPane.TogglePropertyState("Same options in new row", "Off"); @@ -330,7 +362,7 @@ describe( checkNoOptionState(); }); - it("9. should check that server side filering is working", () => { + it("10. should check that server side filering is working", () => { _.dataSources.CreateDataSource("Postgres"); _.dataSources.CreateQueryAfterDSSaved( "SELECT * FROM public.astronauts {{this.params.filterText ? `WHERE name LIKE '%${this.params.filterText}%'` : ''}} LIMIT 10;", diff --git a/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx b/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx index 89c9a1fe15e..fcca7cde3d4 100644 --- a/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx +++ b/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx @@ -189,6 +189,15 @@ export const SelectCell = (props: SelectProps) => { .map((d: DropdownOption) => d.value) .indexOf(value); + const getCellLabelValue = useCallback(() => { + let cellLabelValue: string | number | undefined | null = value; + const selectedOption = options.find((option) => option["value"] === value); + cellLabelValue = selectedOption ? selectedOption["label"] : ""; + return cellLabelValue || ""; + }, [value, options]); + + const cellContent = getCellLabelValue(); + if (isEditable && isCellEditable && isCellEditMode) { return ( { resetFilterTextOnClose={resetFilterTextOnClose} selectedIndex={selectedIndex} serverSideFiltering={serverSideFiltering} - value={value} + value={cellContent} widgetId={""} width={width} /> @@ -257,7 +266,7 @@ export const SelectCell = (props: SelectProps) => { tableWidth={tableWidth} textColor={textColor} textSize={textSize} - value={value} + value={cellContent} verticalAlignment={verticalAlignment} /> ); From af16d61aad4c6e19be7c0792e3d755a64c2f8332 Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Wed, 24 Jul 2024 10:25:37 +0100 Subject: [PATCH 02/13] change string to enum + change useCallback to useMemo in cellLabelValue --- .../TableWidgetV2/component/Constants.ts | 5 ++++ .../component/cellComponents/SelectCell.tsx | 24 ++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/app/client/src/widgets/TableWidgetV2/component/Constants.ts b/app/client/src/widgets/TableWidgetV2/component/Constants.ts index 606c60307f0..a52664b6622 100644 --- a/app/client/src/widgets/TableWidgetV2/component/Constants.ts +++ b/app/client/src/widgets/TableWidgetV2/component/Constants.ts @@ -571,3 +571,8 @@ export const noOfItemsToDisplay = 4; // 12px for the (noOfItemsToDisplay+ 1) item to let the user know there are more items to scroll export const extraSpace = 12; + +export enum TableSelectColumnOptionKeys { + LABEL = "label", + VALUE = "value", +} diff --git a/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx b/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx index fcca7cde3d4..9b6f9da225f 100644 --- a/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx +++ b/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx @@ -1,9 +1,13 @@ -import React from "react"; +import React, { useMemo } from "react"; import SelectComponent from "widgets/SelectWidget/component"; import styled from "styled-components"; import type { DropdownOption } from "widgets/SelectWidget/constants"; import type { BaseCellComponentProps } from "../Constants"; -import { EDITABLE_CELL_PADDING_OFFSET, TABLE_SIZES } from "../Constants"; +import { + EDITABLE_CELL_PADDING_OFFSET, + TABLE_SIZES, + TableSelectColumnOptionKeys, +} from "../Constants"; import { CellWrapper } from "../TableStyledWrappers"; import type { EditableCellActions } from "widgets/TableWidgetV2/constants"; import { BasicCell } from "./BasicCell"; @@ -189,15 +193,17 @@ export const SelectCell = (props: SelectProps) => { .map((d: DropdownOption) => d.value) .indexOf(value); - const getCellLabelValue = useCallback(() => { + const cellLabelValue = useMemo(() => { let cellLabelValue: string | number | undefined | null = value; - const selectedOption = options.find((option) => option["value"] === value); - cellLabelValue = selectedOption ? selectedOption["label"] : ""; + const selectedOption = options.find( + (option) => option[TableSelectColumnOptionKeys.VALUE] === value, + ); + cellLabelValue = selectedOption + ? selectedOption[TableSelectColumnOptionKeys.LABEL] + : ""; return cellLabelValue || ""; }, [value, options]); - const cellContent = getCellLabelValue(); - if (isEditable && isCellEditable && isCellEditMode) { return ( { resetFilterTextOnClose={resetFilterTextOnClose} selectedIndex={selectedIndex} serverSideFiltering={serverSideFiltering} - value={cellContent} + value={cellLabelValue} widgetId={""} width={width} /> @@ -266,7 +272,7 @@ export const SelectCell = (props: SelectProps) => { tableWidth={tableWidth} textColor={textColor} textSize={textSize} - value={cellContent} + value={cellLabelValue} verticalAlignment={verticalAlignment} /> ); From c0f3773a618202498194ffb28aa06e34709abae3 Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Wed, 24 Jul 2024 17:13:45 +0100 Subject: [PATCH 03/13] add error proof feature flag --- app/client/src/ce/entities/FeatureFlag.ts | 3 ++ .../component/cellComponents/SelectCell.tsx | 33 +++++++++++-------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/app/client/src/ce/entities/FeatureFlag.ts b/app/client/src/ce/entities/FeatureFlag.ts index b4eb7da2022..6deb49556d8 100644 --- a/app/client/src/ce/entities/FeatureFlag.ts +++ b/app/client/src/ce/entities/FeatureFlag.ts @@ -34,6 +34,8 @@ export const FEATURE_FLAG = { "rollout_remove_feature_walkthrough_enabled", release_drag_drop_building_blocks_enabled: "release_drag_drop_building_blocks_enabled", + release_table_cell_label_value_enabled: + "release_table_cell_label_value_enabled", rollout_js_enabled_one_click_binding_enabled: "rollout_js_enabled_one_click_binding_enabled", rollout_side_by_side_enabled: "rollout_side_by_side_enabled", @@ -63,6 +65,7 @@ export const DEFAULT_FEATURE_FLAG_VALUE: FeatureFlags = { license_gac_enabled: false, release_anvil_enabled: false, release_drag_drop_building_blocks_enabled: false, + release_table_cell_label_value_enabled: false, license_git_branch_protection_enabled: false, release_git_autocommit_feature_enabled: false, license_git_continuous_delivery_enabled: false, diff --git a/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx b/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx index 9b6f9da225f..89d986ae2e7 100644 --- a/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx +++ b/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx @@ -1,7 +1,10 @@ -import React, { useMemo } from "react"; -import SelectComponent from "widgets/SelectWidget/component"; +import { FEATURE_FLAG } from "@appsmith/entities/FeatureFlag"; +import React, { useCallback, useMemo } from "react"; import styled from "styled-components"; +import { useFeatureFlag } from "utils/hooks/useFeatureFlag"; +import SelectComponent from "widgets/SelectWidget/component"; import type { DropdownOption } from "widgets/SelectWidget/constants"; +import type { EditableCellActions } from "widgets/TableWidgetV2/constants"; import type { BaseCellComponentProps } from "../Constants"; import { EDITABLE_CELL_PADDING_OFFSET, @@ -9,9 +12,7 @@ import { TableSelectColumnOptionKeys, } from "../Constants"; import { CellWrapper } from "../TableStyledWrappers"; -import type { EditableCellActions } from "widgets/TableWidgetV2/constants"; import { BasicCell } from "./BasicCell"; -import { useCallback } from "react"; const StyledSelectComponent = styled(SelectComponent)<{ accentColor: string; @@ -193,16 +194,22 @@ export const SelectCell = (props: SelectProps) => { .map((d: DropdownOption) => d.value) .indexOf(value); + const releaseTableSelectCellLabelValue = useFeatureFlag( + FEATURE_FLAG.release_table_cell_label_value_enabled, + ); + const cellLabelValue = useMemo(() => { - let cellLabelValue: string | number | undefined | null = value; - const selectedOption = options.find( - (option) => option[TableSelectColumnOptionKeys.VALUE] === value, - ); - cellLabelValue = selectedOption - ? selectedOption[TableSelectColumnOptionKeys.LABEL] - : ""; - return cellLabelValue || ""; - }, [value, options]); + if (releaseTableSelectCellLabelValue) { + const selectedOption = options.find( + (option) => option[TableSelectColumnOptionKeys.VALUE] === value, + ); + return selectedOption + ? selectedOption[TableSelectColumnOptionKeys.LABEL] + : ""; + } else { + return value; + } + }, [releaseTableSelectCellLabelValue, value, options]); if (isEditable && isCellEditable && isCellEditMode) { return ( From 78b268d2b566aa07ef23fec567ce05e8c5a6add4 Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Wed, 24 Jul 2024 17:33:22 +0100 Subject: [PATCH 04/13] remove redundant cypress test + improve existing test case --- .../TableV2/columnTypes/Select1_spec.ts | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts index 58dbfda12fe..640a79b2ffa 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts @@ -5,6 +5,7 @@ import { const commonlocators = require("../../../../../../locators/commonlocators.json"); import * as _ from "../../../../../../support/Objects/ObjectsCore"; +import { featureFlagIntercept } from "../../../../../../support/Objects/FeatureFlags"; describe( "Table widget - Select column type functionality", @@ -153,6 +154,7 @@ describe( }); it("5. should check that on option select is working", () => { + featureFlagIntercept({ release_table_cell_label_value_enabled: true }); cy.openPropertyPane("tablewidgetv2"); cy.editColumn("step"); cy.get(".t--property-control-onoptionchange .t--js-toggle").click(); @@ -162,19 +164,6 @@ describe( {{showAlert(currentRow.step)}} `, ); - cy.editTableSelectCell(0, 0); - cy.get(".menu-item-link").contains("#3").click(); - - _.agHelper.ValidateToastMessage("#3"); - - cy.get(".menu-virtual-list").should("not.exist"); - cy.readTableV2data(0, 0).then((val) => { - expect(val).to.equal("#3"); - }); - cy.discardTableRow(4, 0); - }); - - it("6. should check that on option select uses label as cell content in select cell", () => { cy.updateCodeInput( ".t--property-control-options", ` @@ -206,7 +195,7 @@ describe( cy.discardTableRow(4, 0); }); - it("7. should check that currentRow is accessible in the select options", () => { + it("6. should check that currentRow is accessible in the select options", () => { cy.updateCodeInput( ".t--property-control-options", ` @@ -231,7 +220,7 @@ describe( cy.get(".menu-item-text").contains("#1").should("not.exist"); }); - it("8. should check that 'same select option in new row' property is working", () => { + it("7. should check that 'same select option in new row' property is working", () => { _.propPane.NavigateBackToPropertyPane(); const checkSameOptionsInNewRowWhileEditing = () => { @@ -297,7 +286,7 @@ describe( checkSameOptionsWhileAddingNewRow(); }); - it("9. should check that 'new row select options' is working", () => { + it("8. should check that 'new row select options' is working", () => { const checkNewRowOptions = () => { // New row select options should be visible when "Same options in new row" is turned off _.propPane.TogglePropertyState("Same options in new row", "Off"); @@ -362,7 +351,7 @@ describe( checkNoOptionState(); }); - it("10. should check that server side filering is working", () => { + it("9. should check that server side filering is working", () => { _.dataSources.CreateDataSource("Postgres"); _.dataSources.CreateQueryAfterDSSaved( "SELECT * FROM public.astronauts {{this.params.filterText ? `WHERE name LIKE '%${this.params.filterText}%'` : ''}} LIMIT 10;", From b5c3ab87ba6921f0920658499c126cf0ce0280cc Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Thu, 25 Jul 2024 12:04:58 +0100 Subject: [PATCH 05/13] complete sortBy logic in getFilteredTableData --- .../TableWidgetV2/component/Constants.ts | 7 ++++ .../widgets/TableWidgetV2/widget/derived.js | 39 +++++++++++++++++++ .../propertyConfig/PanelConfig/Select.ts | 27 +++++++++++++ 3 files changed, 73 insertions(+) diff --git a/app/client/src/widgets/TableWidgetV2/component/Constants.ts b/app/client/src/widgets/TableWidgetV2/component/Constants.ts index 606c60307f0..e56bb249626 100644 --- a/app/client/src/widgets/TableWidgetV2/component/Constants.ts +++ b/app/client/src/widgets/TableWidgetV2/component/Constants.ts @@ -183,6 +183,7 @@ export interface SelectCellProperties { placeholderText?: string; resetFilterTextOnClose?: boolean; selectOptions?: DropdownOption[]; + sortBy?: string; } export interface ImageCellProperties { @@ -341,6 +342,7 @@ export interface EditActionColumnProperties { placeholderText?: string; resetFilterTextOnClose?: boolean; selectOptions?: DropdownOption[] | DropdownOption[][]; + sortBy?: string; } export interface CurrencyColumnProperties { @@ -571,3 +573,8 @@ export const noOfItemsToDisplay = 4; // 12px for the (noOfItemsToDisplay+ 1) item to let the user know there are more items to scroll export const extraSpace = 12; + +export enum TableSelectColumnOptionKeys { + LABEL = "label", + VALUE = "value", +} diff --git a/app/client/src/widgets/TableWidgetV2/widget/derived.js b/app/client/src/widgets/TableWidgetV2/widget/derived.js index 2b1daa55d1f..76753a80f54 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/derived.js +++ b/app/client/src/widgets/TableWidgetV2/widget/derived.js @@ -265,6 +265,45 @@ export default { let processedTableData = [...props.processedTableData]; const derivedColumns = {}; + /* + Check if there are select columns, + and if the columns are sorting by label instead of default value + */ + const selectColumnKeys = []; + Object.keys(primaryColumns).forEach((id) => { + if ( + primaryColumns[id] && + primaryColumns[id].columnType === "select" && + primaryColumns[id].sortBy && + primaryColumns[id].sortBy === "label" + ) { + selectColumnKeys.push(id); + } + }); + + /* + If there are select columns, + transform the specific columns data to show the label instead of the value for soritng + */ + if (selectColumnKeys.length) { + const transformedValueToLabelData = processedTableData.map((row) => { + const newRow = { ...row }; + selectColumnKeys.forEach((key) => { + const value = row[key]; + const selectOptions = + primaryColumns[key].selectOptions[row.__originalIndex__]; + const option = selectOptions.find((option) => option.value === value); + + if (option) { + newRow[key] = option.label; + } + }); + + return newRow; + }); + processedTableData = transformedValueToLabelData; + } + Object.keys(primaryColumns).forEach((id) => { if (primaryColumns[id] && primaryColumns[id].isDerived) { derivedColumns[id] = primaryColumns[id]; diff --git a/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Select.ts b/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Select.ts index d866fa74a32..eb566e922d4 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Select.ts +++ b/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Select.ts @@ -7,6 +7,7 @@ import { hideByColumnType, selectColumnOptionsValidation, } from "../../propertyUtils"; +import { TableSelectColumnOptionKeys } from "widgets/TableWidgetV2/component/Constants"; export default { sectionName: "Select properties", @@ -37,6 +38,32 @@ export default { return hideByColumnType(props, propertyPath, [ColumnTypes.SELECT]); }, }, + { + propertyName: "sortBy", + defaultValue: "value", + helpText: "Choose whether to sort the select cell by the value or label", + label: "Sort by", + controlType: "DROP_DOWN", + isBindProperty: true, + isJSConvertible: false, + isTriggerProperty: false, + options: [ + { + label: "Label", + value: TableSelectColumnOptionKeys.LABEL, + }, + { + label: "Value", + value: TableSelectColumnOptionKeys.VALUE, + }, + ], + validation: { + type: ValidationTypes.TEXT, + params: { + allowedValues: ["label", "value"], + }, + }, + }, { propertyName: "allowSameOptionsInNewRow", defaultValue: true, From 8de7fa435aaa58cfef67b95453e79194f7ffb75c Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Thu, 25 Jul 2024 16:39:04 +0100 Subject: [PATCH 06/13] return sorted data to standard format after sorting --- .../component/cellComponents/SelectCell.tsx | 5 +- .../widgets/TableWidgetV2/widget/derived.js | 104 +++++++++++------- 2 files changed, 66 insertions(+), 43 deletions(-) diff --git a/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx b/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx index 89d986ae2e7..7f97bdd13bc 100644 --- a/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx +++ b/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx @@ -194,9 +194,8 @@ export const SelectCell = (props: SelectProps) => { .map((d: DropdownOption) => d.value) .indexOf(value); - const releaseTableSelectCellLabelValue = useFeatureFlag( - FEATURE_FLAG.release_table_cell_label_value_enabled, - ); + const releaseTableSelectCellLabelValue = true; + useFeatureFlag(FEATURE_FLAG.release_table_cell_label_value_enabled); const cellLabelValue = useMemo(() => { if (releaseTableSelectCellLabelValue) { diff --git a/app/client/src/widgets/TableWidgetV2/widget/derived.js b/app/client/src/widgets/TableWidgetV2/widget/derived.js index 76753a80f54..eb641376f20 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/derived.js +++ b/app/client/src/widgets/TableWidgetV2/widget/derived.js @@ -265,45 +265,6 @@ export default { let processedTableData = [...props.processedTableData]; const derivedColumns = {}; - /* - Check if there are select columns, - and if the columns are sorting by label instead of default value - */ - const selectColumnKeys = []; - Object.keys(primaryColumns).forEach((id) => { - if ( - primaryColumns[id] && - primaryColumns[id].columnType === "select" && - primaryColumns[id].sortBy && - primaryColumns[id].sortBy === "label" - ) { - selectColumnKeys.push(id); - } - }); - - /* - If there are select columns, - transform the specific columns data to show the label instead of the value for soritng - */ - if (selectColumnKeys.length) { - const transformedValueToLabelData = processedTableData.map((row) => { - const newRow = { ...row }; - selectColumnKeys.forEach((key) => { - const value = row[key]; - const selectOptions = - primaryColumns[key].selectOptions[row.__originalIndex__]; - const option = selectOptions.find((option) => option.value === value); - - if (option) { - newRow[key] = option.label; - } - }); - - return newRow; - }); - processedTableData = transformedValueToLabelData; - } - Object.keys(primaryColumns).forEach((id) => { if (primaryColumns[id] && primaryColumns[id].isDerived) { derivedColumns[id] = primaryColumns[id]; @@ -356,6 +317,45 @@ export default { const sortByColumnId = props.sortOrder.column; let sortedTableData; + /* + Check if there are select columns, + and if the columns are sorting by label instead of default value + */ + const selectColumnKeysWithSortByLabel = []; + Object.keys(primaryColumns).forEach((id) => { + if ( + primaryColumns[id] && + primaryColumns[id].columnType === "select" && + primaryColumns[id].sortBy && + primaryColumns[id].sortBy === "label" + ) { + selectColumnKeysWithSortByLabel.push(id); + } + }); + + /* + If there are select columns, + transform the specific columns data to show the label instead of the value for sorting + */ + let transformedSortTableData; + if (selectColumnKeysWithSortByLabel.length) { + const transformedValueToLabelData = processedTableData.map((row) => { + const newRow = { ...row }; + selectColumnKeysWithSortByLabel.forEach((key) => { + const value = row[key]; + const selectOptions = + primaryColumns[key].selectOptions[row.__originalIndex__]; + const option = selectOptions.find((option) => option.value === value); + + if (option) { + newRow[key] = option.label; + } + }); + + return newRow; + }); + transformedSortTableData = transformedValueToLabelData; + } if (sortByColumnId) { const sortBycolumn = columns.find( @@ -391,7 +391,11 @@ export default { } }; - sortedTableData = processedTableData.sort((a, b) => { + const tableDataForSorting = selectColumnKeysWithSortByLabel.length + ? transformedSortTableData + : processedTableData; + + sortedTableData = tableDataForSorting.sort((a, b) => { if (_.isPlainObject(a) && _.isPlainObject(b)) { if ( isEmptyOrNil(a[sortByColumnOriginalId]) || @@ -444,6 +448,26 @@ export default { return isAscOrder ? 1 : 0; } }); + + if (selectColumnKeysWithSortByLabel.length) { + const finalSortedTableData = sortedTableData.map((row) => { + const newRow = { ...row }; + selectColumnKeysWithSortByLabel.forEach((key) => { + const label = row[key]; + const selectOptions = + primaryColumns[key].selectOptions[row.__originalIndex__]; + const option = selectOptions.find( + (option) => option.label === label, + ); + if (option) { + newRow[key] = option.value; + } + }); + + return newRow; + }); + sortedTableData = finalSortedTableData; + } } else { sortedTableData = [...processedTableData]; } From aca623b4b2c4835cf8b82cb6273f2841dfcecb6f Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Thu, 25 Jul 2024 16:39:43 +0100 Subject: [PATCH 07/13] remove hard coded feature flag --- .../TableWidgetV2/component/cellComponents/SelectCell.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx b/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx index 7f97bdd13bc..89d986ae2e7 100644 --- a/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx +++ b/app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx @@ -194,8 +194,9 @@ export const SelectCell = (props: SelectProps) => { .map((d: DropdownOption) => d.value) .indexOf(value); - const releaseTableSelectCellLabelValue = true; - useFeatureFlag(FEATURE_FLAG.release_table_cell_label_value_enabled); + const releaseTableSelectCellLabelValue = useFeatureFlag( + FEATURE_FLAG.release_table_cell_label_value_enabled, + ); const cellLabelValue = useMemo(() => { if (releaseTableSelectCellLabelValue) { From 7f93e56ec107cb6da877f4171151cda242d20b38 Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Fri, 26 Jul 2024 14:50:01 +0100 Subject: [PATCH 08/13] add test to check sortBy as label and value --- .../Widgets/TableV2/TableV2_Sorting_spec.js | 141 +++++++++++++++++- .../cypress/locators/commonlocators.json | 3 +- 2 files changed, 140 insertions(+), 4 deletions(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js index 2c650e81c0e..b84de3ffca5 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js @@ -1,15 +1,66 @@ +import { featureFlagIntercept } from "../../../../../support/Objects/FeatureFlags"; import * as _ from "../../../../../support/Objects/ObjectsCore"; const testdata = require("../../../../../fixtures/testdata.json"); +const commonlocators = require("../../../../../locators/commonlocators.json"); +import PageList from "../../../../../support/Pages/PageList"; + +const demoTableData = ` + {{ +[ + { + role: 1, + id: 1, + name: "Alice Johnson", + email: "alice.johnson@example.com", + age: 28, + gender: 2 + }, + { + role: 2, + id: 2, + name: "Bob Smith", + email: "bob.smith@example.com", + age: 34, + gender: 1 + }, + { + role: 3, + id: 3, + name: "Charlie Brown", + email: "charlie.brown@example.com", + age: 25, + gender: 3 + }, + { + role: 2, + id: 4, + name: "Diana Prince", + email: "diana.prince@example.com", + age: 30, + gender: 2 + }, + { + role: 1, + id: 5, + name: "Evan Williams", + email: "evan.williams@example.com", + age: 27, + gender: 1 + } +] +}} + `; describe( "Table Widget V2 Sorting", { tags: ["@tag.Widget", "@tag.Table"] }, function () { - before(() => { - _.agHelper.AddDsl("tableV2NewDslWithPagination"); + beforeEach(() => { + PageList.AddNewPage(); }); - it("verifies that table sorting works for a custom column with computed value even when it is renamed", function () { + it("1. Verifies that table sorting works for a custom column with computed value even when it is renamed", function () { + _.agHelper.AddDsl("tableV2NewDslWithPagination"); cy.openPropertyPane("tablewidgetv2"); cy.addColumnV2("customColumn1"); cy.editColumn("customColumn1"); @@ -64,5 +115,89 @@ describe( expect(data).to.eq("8"); }); }); + + it("2. Verifies that default sorting works for a select column using the value property", function () { + // This flag is turned on to allow the label show in the table select cell content + // when this feature is turned on fully, this flag will be removed + featureFlagIntercept({ release_table_cell_label_value_enabled: true }); + cy.dragAndDropToCanvas("tablewidgetv2", { x: 350, y: 500 }); + _.propPane.EnterJSContext("Table data", demoTableData); + + // edit role column to select type + cy.openPropertyPane("tablewidgetv2"); + cy.editColumn("role"); + cy.get(commonlocators.changeColType).last().click(); + cy.get(".t--dropdown-option").children().contains("Select").click(); + cy.wait("@updateLayout"); + + // add dummy select data to the column + cy.get(".t--property-control-options").should("exist"); + cy.updateCodeInput( + ".t--property-control-options", + ` + {{ + [ + {"label": "Software Engineer", + "value": 1,}, + {"label": "Product Manager", + "value": 2,}, + {"label": "UX Designer", + "value": 3,} + ] + }} + `, + ); + cy.backFromPropertyPanel(); + + // sort the column in ascending order + cy.sortColumn("role", "ascending"); + cy.readTableV2data(0, 0).then((data) => { + expect(data).to.eq("Software Engineer"); + }); + }); + + it("3. Verifies that sorting works for the select column type when sortBy is set to label", function () { + // This flag is turned on to allow the label show in the table select cell content + // when this feature is turned on fully, this flag will be removed + featureFlagIntercept({ release_table_cell_label_value_enabled: true }); + cy.dragAndDropToCanvas("tablewidgetv2", { x: 350, y: 500 }); + _.propPane.EnterJSContext("Table data", demoTableData); + + // edit role column to select type + cy.openPropertyPane("tablewidgetv2"); + cy.editColumn("role"); + cy.get(commonlocators.changeColType).last().click(); + cy.get(".t--dropdown-option").children().contains("Select").click(); + + // change sortBy to label + cy.get(commonlocators.changeSortBy).last().click(); + cy.get(".t--dropdown-option").children().contains("Label").click(); + cy.wait("@updateLayout"); + + // add dummy select data to the column + cy.get(".t--property-control-options").should("exist"); + cy.updateCodeInput( + ".t--property-control-options", + ` + {{ + [ + {"label": "Software Engineer", + "value": 1,}, + {"label": "Product Manager", + "value": 2,}, + {"label": "UX Designer", + "value": 3,} + ] + }} + `, + ); + cy.backFromPropertyPanel(); + + // sort the column in ascending order + cy.sortColumn("role", "ascending"); + cy.readTableV2data(0, 0).then((data) => { + expect(data).to.eq("Product Manager"); + }); + }); }, ); diff --git a/app/client/cypress/locators/commonlocators.json b/app/client/cypress/locators/commonlocators.json index 7612ee5c42d..986c006820c 100644 --- a/app/client/cypress/locators/commonlocators.json +++ b/app/client/cypress/locators/commonlocators.json @@ -106,6 +106,7 @@ "editColTitle": ".t--property-pane-title", "editColText": ".t--property-pane-title span", "changeColType": ".t--property-control-columntype input", + "changeSortBy": ".t--property-control-sortby input", "selectedColType": ".t--property-control-columntype button span", "collapsesection": ".t--property-pane-section-collapse-general .bp3-icon", "selectTab": ".t--tab-Tab", @@ -241,4 +242,4 @@ "clientSideSearch": ".t--property-control-clientsidesearch input[type='checkbox']", "enableClientSideSearch": ".t--property-control-enableclientsidesearch input[type='checkbox']", "fixedFooterInput": ".t--property-control-fixedfooter input" -} \ No newline at end of file +} From 6174a55556b50a15a735f50fa677ba111300da6a Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Mon, 29 Jul 2024 07:15:33 +0100 Subject: [PATCH 09/13] better naming + add better check for primaryColumn.selectOptions --- .../widgets/TableWidgetV2/widget/derived.js | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/app/client/src/widgets/TableWidgetV2/widget/derived.js b/app/client/src/widgets/TableWidgetV2/widget/derived.js index eb641376f20..1e3b3334b92 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/derived.js +++ b/app/client/src/widgets/TableWidgetV2/widget/derived.js @@ -327,7 +327,9 @@ export default { primaryColumns[id] && primaryColumns[id].columnType === "select" && primaryColumns[id].sortBy && - primaryColumns[id].sortBy === "label" + primaryColumns[id].sortBy === "label" && + primaryColumns[id].selectOptions && + primaryColumns[id].selectOptions.length ) { selectColumnKeysWithSortByLabel.push(id); } @@ -337,9 +339,9 @@ export default { If there are select columns, transform the specific columns data to show the label instead of the value for sorting */ - let transformedSortTableData; + let processedTableDataWithLabelInseadOfValue; if (selectColumnKeysWithSortByLabel.length) { - const transformedValueToLabelData = processedTableData.map((row) => { + const transformedValueToLabelTableData = processedTableData.map((row) => { const newRow = { ...row }; selectColumnKeysWithSortByLabel.forEach((key) => { const value = row[key]; @@ -354,7 +356,8 @@ export default { return newRow; }); - transformedSortTableData = transformedValueToLabelData; + processedTableDataWithLabelInseadOfValue = + transformedValueToLabelTableData; } if (sortByColumnId) { @@ -391,11 +394,12 @@ export default { } }; - const tableDataForSorting = selectColumnKeysWithSortByLabel.length - ? transformedSortTableData - : processedTableData; + const transformedTableDataForSorting = + selectColumnKeysWithSortByLabel.length + ? processedTableDataWithLabelInseadOfValue + : processedTableData; - sortedTableData = tableDataForSorting.sort((a, b) => { + sortedTableData = transformedTableDataForSorting.sort((a, b) => { if (_.isPlainObject(a) && _.isPlainObject(b)) { if ( isEmptyOrNil(a[sortByColumnOriginalId]) || @@ -450,7 +454,7 @@ export default { }); if (selectColumnKeysWithSortByLabel.length) { - const finalSortedTableData = sortedTableData.map((row) => { + const transformedLabelToValueData = sortedTableData.map((row) => { const newRow = { ...row }; selectColumnKeysWithSortByLabel.forEach((key) => { const label = row[key]; @@ -466,7 +470,7 @@ export default { return newRow; }); - sortedTableData = finalSortedTableData; + sortedTableData = transformedLabelToValueData; } } else { sortedTableData = [...processedTableData]; From cc83a5e1c47853de71402ae6dccc78d3f94480e7 Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Mon, 29 Jul 2024 08:00:32 +0100 Subject: [PATCH 10/13] fix cypress test locators - use commonLocators --- .../Widgets/TableV2/TableV2_Sorting_spec.js | 106 +++++++++--------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js index b84de3ffca5..4b3432d8b54 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js @@ -5,51 +5,51 @@ const commonlocators = require("../../../../../locators/commonlocators.json"); import PageList from "../../../../../support/Pages/PageList"; const demoTableData = ` - {{ -[ - { - role: 1, - id: 1, - name: "Alice Johnson", - email: "alice.johnson@example.com", - age: 28, - gender: 2 - }, - { - role: 2, - id: 2, - name: "Bob Smith", - email: "bob.smith@example.com", - age: 34, - gender: 1 - }, - { - role: 3, - id: 3, - name: "Charlie Brown", - email: "charlie.brown@example.com", - age: 25, - gender: 3 - }, - { - role: 2, - id: 4, - name: "Diana Prince", - email: "diana.prince@example.com", - age: 30, - gender: 2 - }, - { - role: 1, - id: 5, - name: "Evan Williams", - email: "evan.williams@example.com", - age: 27, - gender: 1 - } -] +{{ + [ + { + role: 1, + id: 1, + name: "Alice Johnson", + email: "alice.johnson@example.com", + age: 28, + gender: 2 + }, + { + role: 2, + id: 2, + name: "Bob Smith", + email: "bob.smith@example.com", + age: 34, + gender: 1 + }, + { + role: 3, + id: 3, + name: "Charlie Brown", + email: "charlie.brown@example.com", + age: 25, + gender: 3 + }, + { + role: 2, + id: 4, + name: "Diana Prince", + email: "diana.prince@example.com", + age: 30, + gender: 2 + }, + { + role: 1, + id: 5, + name: "Evan Williams", + email: "evan.williams@example.com", + age: 27, + gender: 1 + } + ] }} - `; + `; describe( "Table Widget V2 Sorting", @@ -88,8 +88,8 @@ describe( // Rename customColumn1 to customColumn2 cy.openPropertyPane("tablewidgetv2"); cy.editColumn("customColumn1"); - cy.get(".t--property-pane-title").click({ force: true }); - cy.get(".t--property-pane-title") + cy.get(_.propPane._paneTitle).click({ force: true }); + cy.get(_.propPane._paneTitle) .type("customColumn2", { delay: 300 }) .type("{enter}"); @@ -127,13 +127,13 @@ describe( cy.openPropertyPane("tablewidgetv2"); cy.editColumn("role"); cy.get(commonlocators.changeColType).last().click(); - cy.get(".t--dropdown-option").children().contains("Select").click(); + cy.get(_.locators._dropdownText).children().contains("Select").click(); cy.wait("@updateLayout"); // add dummy select data to the column - cy.get(".t--property-control-options").should("exist"); + cy.get(_.locators._controlOption).should("exist"); cy.updateCodeInput( - ".t--property-control-options", + _.locators._controlOption, ` {{ [ @@ -167,17 +167,17 @@ describe( cy.openPropertyPane("tablewidgetv2"); cy.editColumn("role"); cy.get(commonlocators.changeColType).last().click(); - cy.get(".t--dropdown-option").children().contains("Select").click(); + cy.get(_.locators._dropdownText).children().contains("Select").click(); // change sortBy to label cy.get(commonlocators.changeSortBy).last().click(); - cy.get(".t--dropdown-option").children().contains("Label").click(); + cy.get(_.locators._dropdownText).children().contains("Label").click(); cy.wait("@updateLayout"); // add dummy select data to the column - cy.get(".t--property-control-options").should("exist"); + cy.get(_.locators._controlOption).should("exist"); cy.updateCodeInput( - ".t--property-control-options", + _.locators._controlOption, ` {{ [ From 678a3a4db93d8e65b6f5badefd67f6c74107c996 Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Mon, 29 Jul 2024 22:17:06 +0100 Subject: [PATCH 11/13] fix test cases --- .../Widgets/TableV2/TableV2_Sorting_spec.js | 24 +++++++++++++++++++ .../widgets/TableWidgetV2/widget/derived.js | 15 ++++++------ .../propertyConfig/PanelConfig/Select.ts | 5 +++- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js index 4b3432d8b54..facb855e796 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Sorting_spec.js @@ -154,6 +154,18 @@ describe( cy.readTableV2data(0, 0).then((data) => { expect(data).to.eq("Software Engineer"); }); + cy.readTableV2data(1, 0).then((data) => { + expect(data).to.eq("Software Engineer"); + }); + cy.readTableV2data(2, 0).then((data) => { + expect(data).to.eq("Product Manager"); + }); + cy.readTableV2data(3, 0).then((data) => { + expect(data).to.eq("Product Manager"); + }); + cy.readTableV2data(4, 0).then((data) => { + expect(data).to.eq("UX Designer"); + }); }); it("3. Verifies that sorting works for the select column type when sortBy is set to label", function () { @@ -198,6 +210,18 @@ describe( cy.readTableV2data(0, 0).then((data) => { expect(data).to.eq("Product Manager"); }); + cy.readTableV2data(1, 0).then((data) => { + expect(data).to.eq("Product Manager"); + }); + cy.readTableV2data(2, 0).then((data) => { + expect(data).to.eq("Software Engineer"); + }); + cy.readTableV2data(3, 0).then((data) => { + expect(data).to.eq("Software Engineer"); + }); + cy.readTableV2data(4, 0).then((data) => { + expect(data).to.eq("UX Designer"); + }); }); }, ); diff --git a/app/client/src/widgets/TableWidgetV2/widget/derived.js b/app/client/src/widgets/TableWidgetV2/widget/derived.js index 1e3b3334b92..980177d3d6d 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/derived.js +++ b/app/client/src/widgets/TableWidgetV2/widget/derived.js @@ -323,14 +323,13 @@ export default { */ const selectColumnKeysWithSortByLabel = []; Object.keys(primaryColumns).forEach((id) => { - if ( - primaryColumns[id] && - primaryColumns[id].columnType === "select" && - primaryColumns[id].sortBy && - primaryColumns[id].sortBy === "label" && - primaryColumns[id].selectOptions && - primaryColumns[id].selectOptions.length - ) { + const column = primaryColumns[id]; + const isColumnSortedByLabel = + column && + column.columnType === "select" && + column?.sortBy === "label" && + column?.selectOptions?.length; + if (isColumnSortedByLabel) { selectColumnKeysWithSortByLabel.push(id); } }); diff --git a/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Select.ts b/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Select.ts index eb566e922d4..a87ff8087b4 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Select.ts +++ b/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Select.ts @@ -60,7 +60,10 @@ export default { validation: { type: ValidationTypes.TEXT, params: { - allowedValues: ["label", "value"], + allowedValues: [ + TableSelectColumnOptionKeys.LABEL, + TableSelectColumnOptionKeys.VALUE, + ], }, }, }, From 7c90156e46c016dd740e051622a90ec8bf22771f Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Tue, 30 Jul 2024 10:20:00 +0100 Subject: [PATCH 12/13] improve selectColumnKeysWithSortByLabel logic --- app/client/src/widgets/TableWidgetV2/widget/derived.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/client/src/widgets/TableWidgetV2/widget/derived.js b/app/client/src/widgets/TableWidgetV2/widget/derived.js index 980177d3d6d..94c66abb09d 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/derived.js +++ b/app/client/src/widgets/TableWidgetV2/widget/derived.js @@ -322,11 +322,9 @@ export default { and if the columns are sorting by label instead of default value */ const selectColumnKeysWithSortByLabel = []; - Object.keys(primaryColumns).forEach((id) => { - const column = primaryColumns[id]; + Object.entries(primaryColumns).forEach(([id, column]) => { const isColumnSortedByLabel = - column && - column.columnType === "select" && + column?.columnType === "select" && column?.sortBy === "label" && column?.selectOptions?.length; if (isColumnSortedByLabel) { From 8a4e8b238adc0d78726a811b54a209caa4990606 Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Tue, 30 Jul 2024 16:17:01 +0100 Subject: [PATCH 13/13] fix typo in processedTableDataWithLabelInsteadOfValue --- app/client/src/widgets/TableWidgetV2/widget/derived.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/client/src/widgets/TableWidgetV2/widget/derived.js b/app/client/src/widgets/TableWidgetV2/widget/derived.js index 94c66abb09d..49ac8f007c9 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/derived.js +++ b/app/client/src/widgets/TableWidgetV2/widget/derived.js @@ -336,7 +336,7 @@ export default { If there are select columns, transform the specific columns data to show the label instead of the value for sorting */ - let processedTableDataWithLabelInseadOfValue; + let processedTableDataWithLabelInsteadOfValue; if (selectColumnKeysWithSortByLabel.length) { const transformedValueToLabelTableData = processedTableData.map((row) => { const newRow = { ...row }; @@ -353,7 +353,7 @@ export default { return newRow; }); - processedTableDataWithLabelInseadOfValue = + processedTableDataWithLabelInsteadOfValue = transformedValueToLabelTableData; } @@ -393,7 +393,7 @@ export default { const transformedTableDataForSorting = selectColumnKeysWithSortByLabel.length - ? processedTableDataWithLabelInseadOfValue + ? processedTableDataWithLabelInsteadOfValue : processedTableData; sortedTableData = transformedTableDataForSorting.sort((a, b) => {