Skip to content

Commit

Permalink
fix: isRequired validation property for table select column (#36375)
Browse files Browse the repository at this point in the history
## Description

**Problem**
The select column of the table widget does not have a validation
property within its property pane to allow users add an isRequired
validation to the table select column.

**Solution**
Added a Validation section to the table select column's property pane,
which includes an isRequired toggle. When enabled, this feature will
trigger a visual indication (error border colour) around the select
widget if a required field is left unselected during "Add new row" or
inline editing.


Fixes #30091 

## Automation

/ok-to-test tags="@tag.Widget, @tag.Table, @tag.Binding, @tag.Sanity,
@tag.Select"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10957896180>
> Commit: d2597e6
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10957896180&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Widget, @tag.Table, @tag.Binding, @tag.Sanity,
@tag.Select`
> Spec:
> <hr>Fri, 20 Sep 2024 12:23:29 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Introduced end-to-end tests for Select column validation in Table
widgets.
- Enhanced validation logic to support Select column types in the Table
widget.
- Added visual feedback for required Select fields during row addition
and inline editing.
- Improved locator for single-select widget button control to enhance UI
interaction.

- **Bug Fixes**
- Improved error handling and visual representation for invalid editable
Select cells.

- **Documentation**
- Updated validation configuration to include Select column types for
better usability.

- **Refactor**
	- Enhanced code clarity for styled components related to Select fields.
- Modified method to improve versatility in handling table interactions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Sai Charan <[email protected]>
Co-authored-by: Pawan Kumar <[email protected]>
  • Loading branch information
3 people authored Sep 23, 2024
1 parent eceaa43 commit 8fe96c9
Show file tree
Hide file tree
Showing 8 changed files with 163 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import commonlocators from "../../../../../../locators/commonlocators.json";
import {
agHelper,
entityExplorer,
locators,
propPane,
table,
} from "../../../../../../support/Objects/ObjectsCore";
import EditorNavigation, {
EntityType,
} from "../../../../../../support/Pages/EditorNavigation";

const TABLE_SELECT_WIDGET_ERROR_BORDER = "rgb(217, 25, 33)";
const TABLE_SELECT_WIDGET_VALID_BORDER = "rgb(85, 61, 233)";

describe(
"Table widget - Select column validation",
{ tags: ["@tag.Widget", "@tag.Table", "@tag.Select"] },
() => {
before(() => {
entityExplorer.DragNDropWidget("tablewidgetv2", 350, 500);
table.AddSampleTableData();
});
it("1. should prevent adding a row when a required select column has no data", () => {
EditorNavigation.SelectEntityByName("Table1", EntityType.Widget);

// Allow adding a row in table
propPane.TogglePropertyState("Allow adding a row", "On");

// Edit step column to select type
table.ChangeColumnType("step", "Select", "v2");
table.EditColumn("step", "v2");

// Add data to select options
agHelper.UpdateCodeInput(
locators._controlOption,
`
[
{
"label": "#1",
"value": "#1"
},
{
"label": "#2",
"value": "#2"
},
{
"label": "#3",
"value": "#3"
}
]
`,
);

// Set step column to editable
propPane.TogglePropertyState("Editable", "On");

// Set step column to required
propPane.TogglePropertyState("Required", "On");

// Click add a new row
table.AddNewRow();

// Expect the save row button to be disabled
agHelper.GetElement(table._saveNewRow).should("be.disabled");

// Expect select to have an error color
agHelper
.GetElement(commonlocators.singleSelectWidgetButtonControl)
.should("have.css", "border-color", TABLE_SELECT_WIDGET_ERROR_BORDER);

// Select a valid option from the select table cell
agHelper.GetNClick(commonlocators.singleSelectWidgetButtonControl);
agHelper
.GetElement(commonlocators.singleSelectWidgetMenuItem)
.contains("#1")
.click();

// Expect the save row option to be enabled
agHelper.GetElement(table._saveNewRow).should("be.enabled");

// Expect button to have a valid color
agHelper
.GetElement(commonlocators.singleSelectWidgetButtonControl)
.should("have.css", "border-color", TABLE_SELECT_WIDGET_VALID_BORDER);

// Discard save new row
agHelper.GetElement(table._discardRow).click({ force: true });
});

it("2. should display an error when inline editing a required select cell in a table with no data", () => {
// Update table data to create emtpy cell in step column
propPane.NavigateBackToPropertyPane();
propPane.UpdatePropertyFieldValue(
"Table data",
`
[
{
"task": "Drop a table",
"status": "✅",
"action": ""
},
{
"step": "#2",
"task": "Create a query fetch_users with the Mock DB",
"status": "--",
"action": ""
},
{
"step": "#3",
"task": "Bind the query using => fetch_users.data",
"status": "--",
"action": ""
}
]
`,
);

// Click the first cell in the step column
table.ClickOnEditIcon(0, 0, true);

// Exect the select to have an error color
agHelper
.GetElement(commonlocators.singleSelectWidgetButtonControl)
.should("have.css", "border-color", TABLE_SELECT_WIDGET_ERROR_BORDER);
});
},
);
1 change: 1 addition & 0 deletions app/client/cypress/locators/commonlocators.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
"callApi": ".t--property-control-onpagechange .t--open-dropdown-Select-Action",
"singleSelectMenuItem": ".bp3-menu-item.single-select div",
"singleSelectWidgetMenuItem": ".menu-item-link",
"singleSelectWidgetButtonControl": ".bp3-button.select-button",
"singleSelectActiveMenuItem": ".menu-item-active div",
"selectInputSearch": ".select-popover-wrapper .bp3-input",
"multiSelectMenuItem": "rc-select-item.rc-select-item-option div",
Expand Down
18 changes: 12 additions & 6 deletions app/client/cypress/support/Pages/Table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -638,18 +638,24 @@ export class Table {
this.agHelper.GetNClick(colSettings);
}

public ClickOnEditIcon(rowIndex: number, colIndex: number) {
public ClickOnEditIcon(
rowIndex: number,
colIndex: number,
isSelectColumn: boolean = false,
) {
this.agHelper.HoverElement(this._tableRow(rowIndex, colIndex, "v2"));
this.agHelper.GetNClick(
this._tableRow(rowIndex, colIndex, "v2") + " " + this._editCellIconDiv,
0,
true,
);
this.agHelper.AssertElementVisibility(
this._tableRow(rowIndex, colIndex, "v2") +
" " +
this._editCellEditorInput,
);
if (!isSelectColumn) {
this.agHelper.AssertElementVisibility(
this._tableRow(rowIndex, colIndex, "v2") +
" " +
this._editCellEditorInput,
);
}
}

public EditTableCell(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const StyledSelectComponent = styled(SelectComponent)<{
accentColor: string;
height: number;
isNewRow: boolean;
isValid: boolean;
}>`
&&& {
width: ${(props) =>
Expand All @@ -37,7 +38,6 @@ const StyledSelectComponent = styled(SelectComponent)<{
}
& button.bp3-button {
border-color: #fff;
padding: 0 9px;
min-height: ${(props) => {
return props.isNewRow
Expand Down Expand Up @@ -82,6 +82,7 @@ type SelectProps = BaseCellComponentProps & {
value: string;
width: number;
isEditable: boolean;
isEditableCellValid: boolean;
tableWidth: number;
isCellEditable?: boolean;
isCellEditMode?: boolean;
Expand Down Expand Up @@ -131,6 +132,7 @@ export const SelectCell = (props: SelectProps) => {
isCellEditMode,
isCellVisible,
isEditable,
isEditableCellValid,
isFilterable = false,
isHidden,
isNewRow,
Expand Down Expand Up @@ -236,13 +238,14 @@ export const SelectCell = (props: SelectProps) => {
compactMode
dropDownWidth={width}
filterText={filterText}
hasError={!isEditableCellValid}
height={TABLE_SIZES[compactMode].ROW_HEIGHT}
hideCancelIcon
isFilterable={isFilterable}
isLoading={false}
isNewRow={isNewRow}
isOpen={autoOpen}
isValid
isValid={isEditableCellValid}
labelText=""
onClose={onClose}
onFilterChange={onFilter}
Expand Down
2 changes: 1 addition & 1 deletion app/client/src/widgets/TableWidgetV2/widget/derived.js
Original file line number Diff line number Diff line change
Expand Up @@ -1037,7 +1037,7 @@ export default {
};

let editableColumns = [];
const validatableColumns = ["text", "number", "currency", "date"];
const validatableColumns = ["text", "number", "currency", "date", "select"];

if (props.isAddRowInProgress) {
Object.values(props.primaryColumns)
Expand Down
1 change: 1 addition & 0 deletions app/client/src/widgets/TableWidgetV2/widget/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2156,6 +2156,7 @@ class TableWidgetV2 extends BaseWidget<TableWidgetProps, WidgetState> {
isCellEditable={isCellEditable}
isCellVisible={cellProperties.isCellVisible ?? true}
isEditable={isColumnEditable}
isEditableCellValid={this.isColumnCellValid(alias)}
isFilterable={cellProperties.isFilterable}
isHidden={isHidden}
isNewRow={isNewRow}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export default {
ColumnTypes.NUMBER,
ColumnTypes.DATE,
ColumnTypes.CURRENCY,
ColumnTypes.SELECT,
],
true,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@ import {
getColumnPath,
} from "widgets/TableWidgetV2/widget/propertyUtils";

const hideColumnByType = (props: TableWidgetProps, propertyPath: string) => {
const path = getColumnPath(propertyPath);

return showByColumnType(
props,
path,
[ColumnTypes.DATE, ColumnTypes.SELECT],
true,
);
};

export default [
{
propertyName: "validation.regex",
Expand All @@ -18,11 +29,7 @@ export default [
isBindProperty: true,
isTriggerProperty: false,
validation: { type: ValidationTypes.REGEX },
hidden: (props: TableWidgetProps, propertyPath: string) => {
const path = getColumnPath(propertyPath);

return showByColumnType(props, path, [ColumnTypes.DATE], true);
},
hidden: hideColumnByType,
},
{
propertyName: "validation.isColumnEditableCellValid",
Expand All @@ -39,11 +46,7 @@ export default [
default: true,
},
},
hidden: (props: TableWidgetProps, propertyPath: string) => {
const path = getColumnPath(propertyPath);

return showByColumnType(props, path, [ColumnTypes.DATE], true);
},
hidden: hideColumnByType,
},
{
propertyName: "validation.errorMessage",
Expand All @@ -56,11 +59,7 @@ export default [
isBindProperty: true,
isTriggerProperty: false,
validation: { type: ValidationTypes.TEXT },
hidden: (props: TableWidgetProps, propertyPath: string) => {
const path = getColumnPath(propertyPath);

return showByColumnType(props, path, [ColumnTypes.DATE], true);
},
hidden: hideColumnByType,
},
{
propertyName: "validation.isColumnEditableCellRequired",
Expand Down

0 comments on commit 8fe96c9

Please sign in to comment.