#1644 Add button for VirtualizedTable Rows in Expression Builder #1645

veenas1 · 2023-12-13T10:03:59Z

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

...dules/common-canvas/src/common-properties/components/virtualized-table/virtualized-table.jsx

canvas_modules/harness/src/client/constants/json/functionlist.json

...ommon-properties/controls/expression/expression-builder/expression-select-field-function.jsx

canvas_modules/common-canvas/__tests__/test_resources/json/expression-function-list.json

canvas_modules/common-canvas/__tests__/common-properties/controls/expression-test.js

matthoward366

Can we do something to make it so the add button row stays the same width? In some case it's using too much space and caused an ellipsis.

Also, I found a defect when sorting a column then clicking on it. The wrong row is selected. This isn't caused by your change but can you have a look to see if it's an easy fix or create a new issue for it.

veenas1 · 2023-12-25T03:51:59Z

@matthoward366

Add column width: Could you please check now, I have made the width to be in px, so that it's used as is and not adjusted.
Column Sort: Unable to replicate the issue. If you can still see the defect, I can work on it with a new issue.

matthoward366 · 2024-01-03T20:25:36Z

...ommon-properties/controls/expression/expression-builder/expression-select-field-function.jsx

+				{
+					key: "addColumn",
+					label: formatMessage(this.reactIntl, MESSAGE_KEYS.EXPRESSION_ADD_COLUMN),
+					width: "50px",


For all the addColumn fields we should also add addColumn staticWidth: true

matthoward366 · 2024-01-03T20:25:45Z

...ommon-properties/controls/expression/expression-builder/expression-select-field-function.jsx

+				{
+					key: "values",
+					label: categoryInfo.field_columns.value_column_info.locLabel,
+					description: categoryInfo.field_columns.value_column_info.descLabel,


Remove comma at the end

matthoward366 · 2024-01-03T20:27:11Z

...ommon-properties/controls/expression/expression-builder/expression-select-field-function.jsx

-
-		headers.push({ key: "function", label: functionColumn, width: 73, resizable: true });
-		headers.push({ key: "return", label: returnColumn, width: 27 });
+		headers.push({ key: "addColumn", label: addNewColumn, width: "50px", resizable: true });


This shouldn't have resizable

matthoward366 · 2024-01-03T20:32:15Z

Here is what I'm seeing for sort. The selection gets off.

Signed-off-by: Veena S <[email protected]>

...dules/common-canvas/src/common-properties/components/virtualized-table/virtualized-table.jsx

matthoward366 · 2024-01-05T22:52:29Z

...dules/common-canvas/src/common-properties/components/virtualized-table/virtualized-table.jsx

@@ -380,7 +380,7 @@ class VirtualizedTable extends React.Component {
 		}

 		if (this.props.selectable) {
-			const rowSelected = this.isRowSelected(rowData.originalRowIndex);
+			const rowSelected = this.props.sortDirection ? this.isRowSelected(rowData.index) : this.isRowSelected(rowData.originalRowIndex); // use current row index when Sorted


@caritaou Do you see any issue with this change? I did some testing around row selection and for the most part it doesn't really work correctly onSort in the table controls. It just clears the selection.

but this does fix the issue in the expressionBuilder around sorting and selection.

This looks good to me, considering this never worked for sorting columns with row selections.

matthoward366

lgtm. @caritaou can you merge if you think the selection change is good.

veenas1 requested review from JesusGuerrero and matthoward366 December 13, 2023 10:04

matthoward366 requested changes Dec 14, 2023

View reviewed changes

veenas1 requested a review from matthoward366 December 15, 2023 11:15

matthoward366 reviewed Dec 15, 2023

View reviewed changes

canvas_modules/common-canvas/__tests__/test_resources/json/expression-function-list.json Outdated Show resolved Hide resolved

matthoward366 reviewed Dec 15, 2023

View reviewed changes

canvas_modules/common-canvas/__tests__/common-properties/controls/expression-test.js Outdated Show resolved Hide resolved

veenas1 requested a review from matthoward366 December 16, 2023 06:24

matthoward366 reviewed Dec 19, 2023

View reviewed changes

veenas1 force-pushed the 8415_add_column_virtualized_table branch from 000a423 to bf56d13 Compare December 25, 2023 03:44

matthoward366 reviewed Jan 3, 2024

View reviewed changes

veenas1 added 7 commits January 4, 2024 10:49

elyra-ai#1644 Add button for VirtualizedTable Rows in Expression Builder

082d321

Signed-off-by: Veena S <[email protected]>

elyra-ai#1644 Add button for VirtualizedTable Rows in Expression Builder

c628daf

Signed-off-by: Veena S <[email protected]>

elyra-ai#1644 Fix Cypress Testcase

734bcd6

Signed-off-by: Veena S <[email protected]>

elyra-ai#1644 Revert schema changes & Code optimize

9b9e117

Signed-off-by: Veena S <[email protected]>

elyra-ai#1644 Fixed test field changes

8b6ebe1

Signed-off-by: Veena S <[email protected]>

elyra-ai#1644 Make Add column width fixed by setting in px

e1b04f3

Signed-off-by: Veena S <[email protected]>

elyra-ai#1644 set addColumn staticWidth true

c7a0d97

Signed-off-by: Veena S <[email protected]>

veenas1 force-pushed the 8415_add_column_virtualized_table branch from 6cbcc4d to c7a0d97 Compare January 4, 2024 05:19

elyra-ai#1644 fix selection off when sorted

b90cdb1

Signed-off-by: Veena S <[email protected]>

veenas1 commented Jan 4, 2024

View reviewed changes

...dules/common-canvas/src/common-properties/components/virtualized-table/virtualized-table.jsx Show resolved Hide resolved

veenas1 requested a review from matthoward366 January 4, 2024 10:45

matthoward366 reviewed Jan 5, 2024

View reviewed changes

matthoward366 approved these changes Jan 5, 2024

View reviewed changes

caritaou merged commit cdfe00b into elyra-ai:main Jan 5, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#1644 Add button for VirtualizedTable Rows in Expression Builder #1645

#1644 Add button for VirtualizedTable Rows in Expression Builder #1645

veenas1 commented Dec 13, 2023

matthoward366 left a comment

veenas1 commented Dec 25, 2023 •

edited

Loading

matthoward366 Jan 3, 2024

matthoward366 Jan 3, 2024

matthoward366 Jan 3, 2024

matthoward366 commented Jan 3, 2024

matthoward366 Jan 5, 2024

matthoward366 Jan 5, 2024

caritaou Jan 5, 2024

matthoward366 left a comment

#1644 Add button for VirtualizedTable Rows in Expression Builder #1645

#1644 Add button for VirtualizedTable Rows in Expression Builder #1645

Conversation

veenas1 commented Dec 13, 2023

matthoward366 left a comment

Choose a reason for hiding this comment

veenas1 commented Dec 25, 2023 • edited Loading

matthoward366 Jan 3, 2024

Choose a reason for hiding this comment

matthoward366 Jan 3, 2024

Choose a reason for hiding this comment

matthoward366 Jan 3, 2024

Choose a reason for hiding this comment

matthoward366 commented Jan 3, 2024

matthoward366 Jan 5, 2024

Choose a reason for hiding this comment

matthoward366 Jan 5, 2024

Choose a reason for hiding this comment

caritaou Jan 5, 2024

Choose a reason for hiding this comment

matthoward366 left a comment

Choose a reason for hiding this comment

veenas1 commented Dec 25, 2023 •

edited

Loading