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.

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

feat: action redesign, UQI upgrade S3 plugin config to dual zone format & sorting field responsiveness #36090

Merged
merged 5 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
219 changes: 98 additions & 121 deletions app/client/src/components/formControls/SortingControl.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import React, { useEffect, useRef } from "react";
import React, { useEffect, useMemo, useRef } from "react";
import { useSelector } from "react-redux";
import FormControl from "pages/Editor/FormControl";
import { Classes } from "@appsmith/ads-old";
import styled from "styled-components";
import { FieldArray, getFormValues } from "redux-form";
import {
FieldArray,
getFormValues,
type WrappedFieldArrayProps,
} from "redux-form";
import type { ControlProps } from "./BaseControl";
import { getBindingOrConfigPathsForSortingControl } from "entities/Action/actionProperties";
import { SortingSubComponent } from "./utils";
Expand All @@ -26,9 +29,6 @@ const columnFieldConfig: any = {
initialValue: "",
inputType: "TEXT",
placeholderText: "Column name",
customStyles: {
// width: "280px",
},
};

// Form config for the order field
Expand All @@ -52,174 +52,151 @@ const orderFieldConfig: any = {
],
};

// main container for the fsorting component
const SortingContainer = styled.div`
display: flex;
flex-direction: column;
justify-content: space-between;
const SortingContainer = styled.div<{ isBreakpointSmall: boolean }>`
display: grid;
grid-template-columns: ${({ isBreakpointSmall }) =>
isBreakpointSmall ? "1fr 50px" : "1fr 50px"};
gap: 5px;
align-items: center;
`;

// container for the two sorting dropdown
const SortingDropdownContainer = styled.div<{ size: string }>`
display: flex;
flex-direction: row;
width: min-content;
justify-content: space-between;
margin-bottom: 5px;
const SortingFields = styled.div<{ isBreakpointSmall: boolean }>`
display: grid;
grid-template-columns: ${({ isBreakpointSmall }) =>
isBreakpointSmall ? "1fr" : "1fr 180px"};
grid-template-rows: ${({ isBreakpointSmall }) =>
isBreakpointSmall ? "1fr 1fr" : "1fr"};
gap: 5px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A change in this file can impact all those plugin forms which are not yet migrated to the new UI changes we are implementing. Have you cross checked no other plugin form using this component reflects any change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question. The old component does not render very well in SECTION or SECTION_V2. Additionally it has issues with typings, dead code and composition that makes little sense. That's why I've decided that it could use some help. Now it renders well, does not break on smaller widths, regardless of type of container, though it still has numerous issues with caching and typings, which are beyond the scope of this task.

Screenshots of both versions in regular SECTION:

Refactored component:
Screenshot 2024-09-06 at 14 02 23

Original component:
Screenshot 2024-09-06 at 14 02 02

align-items: center;
> div {
width: 250px;
}
${(props) =>
props.size === "small" &&
`
// Hide the dropdown labels to decrease the width
// The design system component has inline style hence the !important
.t--form-control-DROP_DOWN .${Classes.TEXT} {
display: none !important;
}
// Show the icons hidden initially
.t--form-control-DROP_DOWN .remixicon-icon {
display: initial;
}
`}
`;

const ButtonWrapper = styled.div`
display: flex;
flex-direction: row;
`;
// container for the column dropdown section
const ColumnDropdownContainer = styled.div``;

// Component for the icons
const CenteredButton = styled(Button)``;
export type SortingControlProps = ControlProps;

// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function SortingComponent(props: any) {
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const formValues: any = useSelector((state) =>
export type SortingComponentProps = WrappedFieldArrayProps &
Pick<SortingControlProps, "configProperty" | "formName">;

function SortingComponent(props: SortingComponentProps) {
const { configProperty, fields, formName } = props;

const formValues = useSelector((state) =>
getFormValues(props.formName)(state),
);

const onDeletePressed = (index: number) => {
props.fields.remove(index);
fields.remove(index);
};

const targetRef = useRef<HTMLDivElement>(null);
const size = useResponsiveBreakpoints(targetRef, [{ small: 450 }]);
const isBreakpointSmall = size === "small";

useEffect(() => {
// this path represents the path to the sortBy object, wherever the location is in the actionConfiguration object
let sortingObjectPath;
// if the path ends with .data which we expect it to.
if (props.configProperty.endsWith(".data")) {
if (configProperty.endsWith(".data")) {
// we remove the .data and get the path of the sort object
// NOTE: 5 is used because (.data) = 5
sortingObjectPath = props.configProperty.substring(
const sortingObjectPath = configProperty.substring(
0,
props.configProperty.length - 5,
configProperty.length - 5,
);
}
// sortDataValue is the path to the value (.data included) itself in the sort object
const sortDataValue = get(formValues, props.configProperty);
// sort object value is the path to the sort object itself.
const sortObjectValue = get(formValues, sortingObjectPath);

// The reason we are making this check is to prevent new fields from being pushed when the form control is visited
// for some reason the fields object is initially undefined in first render, before being initialized with the correct values after.
// so we check to see if the sortObjectValue exist first (if the value has been initalized).
if (!sortObjectValue) {
return;
}

// then we check if the redux fields have any items in it,
// and we also check if the value exists in the redux state as an array and if that value has no items in it.
// if they are both empty we want to push a new field.
// We also want to check if the value is undefined, this means that the sort data value is non existent, if it is, we want to push a new field.
if (
(props.fields.length < 1 &&
isArray(sortDataValue) &&
sortDataValue.length < 1) ||
(props.fields.length < 1 && !sortDataValue)
) {
props.fields.push({
column: "",
order: OrderDropDownValues.ASCENDING,
});
} else {
onDeletePressed(props.index);
// sortDataValue is the path to the value (.data included) itself in the sort object
const sortDataValue = get(formValues, configProperty);
// sort object value is the path to the sort object itself.
const sortObjectValue = get(formValues, sortingObjectPath);

// The reason we are making this check is to prevent new fields from being pushed when the form control is visited
// for some reason the fields object is initially undefined in first render, before being initialized with the correct values after.
// so we check to see if the sortObjectValue exist first (if the value has been initalized).
if (sortObjectValue) {
// then we check if the redux fields have any items in it,
// and we also check if the value exists in the redux state as an array and if that value has no items in it.
// if they are both empty we want to push a new field.
// We also want to check if the value is undefined, this means that the sort data value is non existent, if it is, we want to push a new field.
if (
(fields.length < 1 &&
isArray(sortDataValue) &&
sortDataValue.length < 1) ||
(fields.length < 1 && !sortDataValue)
) {
fields.push({
column: "",
order: OrderDropDownValues.ASCENDING,
});
}
}
}
}, [props.fields.length]);
}, [fields.length]);

return (
<SortingContainer className={`t--${props?.configProperty}`} ref={targetRef}>
{props.fields &&
props.fields.length > 0 &&
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
props.fields.map((field: any, index: number) => {
<SortingContainer
className={`t--${props?.configProperty}`}
isBreakpointSmall={isBreakpointSmall}
ref={targetRef}
>
{fields &&
fields.length > 0 &&
fields.map((field, index: number) => {
const columnPath = getBindingOrConfigPathsForSortingControl(
SortingSubComponent.Column,
field,
undefined,
);
const OrderPath = getBindingOrConfigPathsForSortingControl(

const orderPath = getBindingOrConfigPathsForSortingControl(
SortingSubComponent.Order,
field,
undefined,
);

return (
<SortingDropdownContainer key={index} size={size}>
<ColumnDropdownContainer>
<React.Fragment key={index}>
<SortingFields isBreakpointSmall={isBreakpointSmall}>
<FormControl
config={{
...columnFieldConfig,
customStyles: {
width: "250px",
width: "100%",
},
configProperty: `${columnPath}`,
configProperty: columnPath,
nestedFormControl: true,
}}
formName={props.formName}
formName={formName}
/>
</ColumnDropdownContainer>
<FormControl
config={{
...orderFieldConfig,
configProperty: `${OrderPath}`,
nestedFormControl: true,
customStyles: {
width: isBreakpointSmall ? "65px" : "250px",
},
optionWidth: isBreakpointSmall ? "250px" : undefined,
}}
formName={props.formName}
/>
{/* Component to render the delete icon */}
<CenteredButton
<FormControl
config={{
...orderFieldConfig,
customStyles: {
maxWidth: "180px",
},
configProperty: orderPath,
nestedFormControl: true,
}}
formName={formName}
/>
</SortingFields>
<Button
data-testid={`t--sorting-delete-[${index}]`}
isIconButton
kind="tertiary"
onClick={(e: React.MouseEvent) => {
e.stopPropagation();
onClick={() => {
onDeletePressed(index);
}}
size="md"
startIcon="close-line"
value={index}
/>
</SortingDropdownContainer>
</React.Fragment>
);
})}
<ButtonWrapper>
<Button
data-testid={`t--sorting-add-field`}
kind="tertiary"
onClick={() =>
props.fields.push({
fields.push({
column: "",
order: OrderDropDownValues.ASCENDING,
})
Expand All @@ -240,18 +217,18 @@ export default function SortingControl(props: SortingControlProps) {
formName, // Name of the form, used by redux-form lib to store the data in redux store
} = props;

const fieldArrayProps = useMemo(
() => ({ configProperty, formName }),
[configProperty, formName],
);

return (
<FieldArray
component={SortingComponent}
key={`${configProperty}`}
name={`${configProperty}`}
props={{
configProperty,
formName,
}}
key={configProperty}
name={configProperty}
props={fieldArrayProps}
rerenderOnEveryChange={false}
/>
);
}

export type SortingControlProps = ControlProps;
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
{
"identifier": "UPLOAD_FILE_FROM_BODY",
"controlType": "SECTION",
"controlType": "SECTION_V2",
"conditionals": {
"show": "{{actionConfiguration.formData.command.data === 'UPLOAD_FILE_FROM_BODY'}}"
},
"children": [
{
"controlType": "SECTION",
"label": "Select bucket to query",
"controlType": "DOUBLE_COLUMN_ZONE",
"children": [
{
"label": "Bucket name",
Expand All @@ -16,12 +15,17 @@
"evaluationSubstitutionType": "TEMPLATE",
"isRequired": true,
"initialValue": ""
},
{
"label": "Expiry duration of signed URL (minutes)",
"configProperty": "actionConfiguration.formData.create.expiry.data",
"controlType": "QUERY_DYNAMIC_INPUT_TEXT",
"initialValue": "5"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is completely changing this field's position on the UI. Could you cross-check with the design once? I don't see this field next to the Bucket name field in the design.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your screenshot is for list, the change is for file creation. I will discuss this with Momcilo, because even this screenshot does not adhere to rules, as it looks like the prefix should be next to bucket name.

Bottom line, files have to adhere to rules art boards may be outdated or incorrect.

Screenshot 2024-09-06 at 10 22 06

}
]
},
{
"controlType": "SECTION",
"label": "Query",
"controlType": "DOUBLE_COLUMN_ZONE",
"description": "Optional",
"children": [
{
Expand All @@ -45,13 +49,13 @@
"value": "NO"
}
]
},
{
"label": "Expiry duration of signed URL (minutes)",
"configProperty": "actionConfiguration.formData.create.expiry.data",
"controlType": "QUERY_DYNAMIC_INPUT_TEXT",
"initialValue": "5"
},
}
]
},
{
"controlType": "SINGLE_COLUMN_ZONE",
"description": "Optional",
"children": [
{
"label": "Content",
"configProperty": "actionConfiguration.formData.body.data",
Expand Down
Loading
Loading