Skip to content

chore: add code for paragraph-text morphing #36065

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.

Sign up for GitHub

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

Merged
jsartisan merged 8 commits into from
Sep 6, 2024
Merged

chore: add code for paragraph-text morphing #36065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
260bfc9
add code for paragraph-text morphing
Sep 3, 2024
572f5ae
remove cyclic import
Sep 3, 2024
67e8211
incorporate feedback
Sep 3, 2024
68ce8f2
fix hydration of font style in paragraph widget
Sep 3, 2024
1b1d2ab
fix hydration of font style in paragraph widget
Sep 3, 2024
e1b3c9f
update types
Sep 4, 2024
c8be2f8
add unit test for the hook
Sep 4, 2024
395574a
add unit test for the isReadOnlyUpdateHook
Sep 4, 2024
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
1 change: 0 additions & 1 deletion app/client/src/layoutSystems/anvil/integrations/sagas/anvilWidgetAdditionSagas/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export function getWidgetSessionValues(
}

for (const key in configMap) {
if (configMap[key] === undefined) continue;
Copy link
Contributor Author

@jsartisan jsartisan Sep 3, 2024

Choose a reason for hiding this comment

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

This is not required.

let sessionStorageKey = `${widgetType}.${key}`;

if (type === "ZONE_WIDGET") {
Expand Down
64 changes: 64 additions & 0 deletions app/client/src/widgets/wds/WDSBaseInputWidget/__tests__/helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import type { WidgetProps } from "widgets/BaseWidget";
import { isReadOnlyUpdateHook } from "../helpers";

describe("isReadOnlyUpdateHook", () => {
it("should return the correct updates for input widget", () => {
const props1 = {
type: "WDS_INPUT_WIDGET",
inputType: "TEXT",
} as unknown as WidgetProps;

const updates = isReadOnlyUpdateHook(props1, "readOnly", true);
expect(updates).toEqual([
{ propertyPath: "readOnly", propertyValue: true },
{ propertyPath: "type", propertyValue: "WDS_KEY_VALUE_WIDGET" },
]);

const updates2 = isReadOnlyUpdateHook(props1, "readOnly", false);
expect(updates2).toEqual([
{ propertyPath: "readOnly", propertyValue: false },
{ propertyPath: "type", propertyValue: "WDS_INPUT_WIDGET" },
]);

const props2 = {
type: "WDS_EMAIL_INPUT_WIDGET",
inputType: "EMAIL",
} as unknown as WidgetProps;

const updates3 = isReadOnlyUpdateHook(props2, "readOnly", true);
expect(updates3).toEqual([
{ propertyPath: "readOnly", propertyValue: true },
{ propertyPath: "type", propertyValue: "WDS_KEY_VALUE_WIDGET" },
]);

const updates4 = isReadOnlyUpdateHook(props2, "readOnly", false);
expect(updates4).toEqual([
{ propertyPath: "readOnly", propertyValue: false },
{ propertyPath: "type", propertyValue: "WDS_EMAIL_INPUT_WIDGET" },
]);
});

it("should not change the type for currency widget", () => {
const props = {
type: "WDS_CURRENCY_INPUT_WIDGET",
inputType: "CURRENCY",
} as unknown as WidgetProps;

const updates = isReadOnlyUpdateHook(props, "readOnly", true);
expect(updates).toEqual([
{ propertyPath: "readOnly", propertyValue: true },
]);
});

it("should not change the type for phone widget", () => {
const props = {
type: "WDS_PHONE_INPUT_WIDGET",
inputType: "PHONE",
} as unknown as WidgetProps;

const updates = isReadOnlyUpdateHook(props, "readOnly", true);
expect(updates).toEqual([
{ propertyPath: "readOnly", propertyValue: true },
]);
});
});
Comment on lines +4 to +64
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 5, 2024

Choose a reason for hiding this comment

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

Comprehensive Review of Test Cases for isReadOnlyUpdateHook

General Observations:

  • The file introduces tests for the isReadOnlyUpdateHook function, which appears to handle property updates based on widget type and readOnly status.
  • The tests are well-structured and cover multiple scenarios, including different widget types and readOnly states.

Detailed Observations:

  1. Test Case for Input and Email Widgets (Lines 4-39):

    • The tests correctly simulate different scenarios for input and email widgets, checking the updates when the readOnly property is toggled.
    • The use of as unknown as WidgetProps for type assertion is a bit forceful. Consider using a more refined type or mock that closely resembles WidgetProps without resorting to unknown casting.

  2. Test Case for Currency Widget (Lines 41-51):

    • This test verifies that the type does not change when the widget is set to readOnly, which is a specific behavior presumably defined in the isReadOnlyUpdateHook function.
    • It's good to see that the test expects no change in the type property, which aligns with the intended behavior described.

  3. Test Case for Phone Widget (Lines 53-63):

    • Similar to the currency widget test, this checks for no change in type when readOnly is true.
    • Consistency in testing similar behaviors across different widget types is good for maintainability and understanding of the code.

Suggestions:

  • Refine Type Assertions: Instead of using as unknown as WidgetProps, create a more specific mock or utilize partials of WidgetProps to avoid broad type assertions.
  • Consider Additional Scenarios: Are there edge cases or error conditions that might affect the behavior of isReadOnlyUpdateHook? If so, adding tests to cover these scenarios would be beneficial.

Overall:

  • The tests are well-written and serve their purpose. However, minor improvements in type safety and possibly extending coverage could make them even better.

3 changes: 3 additions & 0 deletions app/client/src/widgets/wds/WDSBaseInputWidget/config/contentConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ValidationTypes } from "constants/WidgetValidation";

import { isReadOnlyUpdateHook } from "../helpers";
import type { BaseInputWidgetProps } from "../widget/types";

export const propertyPaneContentConfig = [
Expand Down Expand Up @@ -121,6 +122,8 @@ export const propertyPaneContentConfig = [
isJSConvertible: true,
isBindProperty: true,
isTriggerProperty: false,
dependencies: ["type", "inputType"],
updateHook: isReadOnlyUpdateHook,
validation: { type: ValidationTypes.BOOLEAN },
},
{
Expand Down
0 ...c/widgets/wds/WDSInputWidget/constants.ts → ...dgets/wds/WDSBaseInputWidget/constants.ts
View file
Open in desktop
File renamed without changes.
35 changes: 35 additions & 0 deletions app/client/src/widgets/wds/WDSBaseInputWidget/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import type { WidgetProps } from "widgets/BaseWidget";
import type { PropertyUpdates } from "WidgetProvider/constants";

import type { InputType } from "./types";
import { INPUT_TYPE_TO_WIDGET_TYPE_MAP } from "./constants";

export function isReadOnlyUpdateHook(
props: WidgetProps,
propertyName: string,
propertyValue: boolean,
) {
const updates: PropertyUpdates[] = [
{
propertyPath: propertyName,
propertyValue: propertyValue,
},
];

// if user is marking readOnly as true and if the input type is not INPUT_CURRENCY_WIDGET or INPUT_PHONE_WIDGET,
// then we update the type to WDS_KEY_VALUE_WIDGET, else we update the type based on the input type
if (
!["WDS_CURRENCY_INPUT_WIDGET", "WDS_PHONE_INPUT_WIDGET"].includes(
props.type,
)
) {
updates.push({
propertyPath: "type",
propertyValue: propertyValue
? "WDS_KEY_VALUE_WIDGET"
: INPUT_TYPE_TO_WIDGET_TYPE_MAP[props.inputType as InputType],
});
}

return updates;
}
3 changes: 3 additions & 0 deletions app/client/src/widgets/wds/WDSBaseInputWidget/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { WDSBaseInputWidget } from "./widget";

export { WDSBaseInputWidget };

export * from "./types";
export * from "./constants";
export type { BaseInputWidgetProps } from "./widget/types";
export type { BaseInputComponentProps } from "./component/types";
3 changes: 3 additions & 0 deletions app/client/src/widgets/wds/WDSBaseInputWidget/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import type { INPUT_TYPES } from "./constants";

export type InputType = (typeof INPUT_TYPES)[keyof typeof INPUT_TYPES];
2 changes: 1 addition & 1 deletion app/client/src/widgets/wds/WDSEmailInputWidget/widget/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { WDSInputWidget } from "widgets/wds/WDSInputWidget";
import { EmailInputIcon, EmailInputThumbnail } from "appsmith-icons";
import { WIDGET_TAGS } from "constants/WidgetConstants";
import { INPUT_TYPES } from "widgets/wds/WDSInputWidget/constants";
import { INPUT_TYPES } from "widgets/wds/WDSBaseInputWidget";
import type { WidgetBaseConfiguration } from "WidgetProvider/constants";

class WDSEmailInputWidget extends WDSInputWidget {
Expand Down
4 changes: 2 additions & 2 deletions app/client/src/widgets/wds/WDSInputWidget/component/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import React from "react";
import { isNil } from "lodash";
import { TextInput } from "@appsmith/wds";
import { Icon, TextArea } from "@appsmith/wds";
import { useDebouncedValue } from "@mantine/hooks";
import { INPUT_TYPES } from "widgets/wds/WDSBaseInputWidget";

import { INPUT_TYPES } from "../constants";
import type { InputComponentProps } from "./types";
import { useDebouncedValue } from "@mantine/hooks";

const DEBOUNCE_TIME = 300;

Expand Down
6 changes: 2 additions & 4 deletions app/client/src/widgets/wds/WDSInputWidget/component/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import type { IconProps } from "@appsmith/wds";
import type { BaseInputComponentProps } from "../../WDSBaseInputWidget";

import type { INPUT_TYPES } from "../constants";
import type { InputType } from "widgets/wds/WDSBaseInputWidget/types";

export type InputType = (typeof INPUT_TYPES)[keyof typeof INPUT_TYPES];
import type { BaseInputComponentProps } from "../../WDSBaseInputWidget";

export interface InputComponentProps extends BaseInputComponentProps {
inputType: InputType;
Expand Down
7 changes: 5 additions & 2 deletions app/client/src/widgets/wds/WDSInputWidget/widget/helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@ import {
INPUT_INVALID_TYPE_ERROR,
INPUT_TEXT_MAX_CHAR_ERROR,
} from "ee/constants/messages";
import type { InputType } from "../component/types";
import type { InputType } from "widgets/wds/WDSBaseInputWidget";
import type { WidgetProps } from "widgets/BaseWidget";

import type { InputWidgetProps, Validation } from "./types";
import { INPUT_TYPE_TO_WIDGET_TYPE_MAP, INPUT_TYPES } from "../constants";
import {
INPUT_TYPE_TO_WIDGET_TYPE_MAP,
INPUT_TYPES,
} from "widgets/wds/WDSBaseInputWidget";
import type { PropertyUpdates } from "WidgetProvider/constants";
import { getDefaultISDCode } from "widgets/wds/WDSPhoneInputWidget/constants";
import { getDefaultCurrency } from "widgets/wds/WDSCurrencyInputWidget/constants";
Expand Down
2 changes: 1 addition & 1 deletion app/client/src/widgets/wds/WDSInputWidget/widget/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from "react";
import { isNumber, merge, toString } from "lodash";
import * as config from "../config";
import InputComponent from "../component";
import { INPUT_TYPES } from "../constants";
import type { InputWidgetProps } from "./types";
import { mergeWidgetConfig } from "utils/helpers";
import { parseText, validateInput } from "./helper";
Expand All @@ -14,6 +13,7 @@ import type { DerivedPropertiesMap } from "WidgetProvider/factory";
import { EventType } from "constants/AppsmithActionConstants/ActionConstants";
import type { KeyDownEvent } from "widgets/wds/WDSBaseInputWidget/component/types";
import type { WidgetBaseConfiguration } from "WidgetProvider/constants";
import { INPUT_TYPES } from "widgets/wds/WDSBaseInputWidget/constants";

class WDSInputWidget extends WDSBaseInputWidget<InputWidgetProps, WidgetState> {
static type = "WDS_INPUT_WIDGET";
Expand Down
4 changes: 2 additions & 2 deletions app/client/src/widgets/wds/WDSMultilineInputWidget/widget/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { WIDGET_TAGS } from "constants/WidgetConstants";
import { WDSInputWidget } from "widgets/wds/WDSInputWidget";
import { MultilineInputIcon, MultilineInputThumbnail } from "appsmith-icons";
import { INPUT_TYPES } from "widgets/wds/WDSInputWidget/constants";
import type { WidgetBaseConfiguration } from "WidgetProvider/constants";
import { INPUT_TYPES } from "widgets/wds/WDSBaseInputWidget/constants";
import { MultilineInputIcon, MultilineInputThumbnail } from "appsmith-icons";

class WDSMultilineInputWidget extends WDSInputWidget {
static type = "WDS_MULTILINE_INPUT_WIDGET";
Expand Down
2 changes: 1 addition & 1 deletion app/client/src/widgets/wds/WDSNumberInputWidget/widget/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { WIDGET_TAGS } from "constants/WidgetConstants";
import { WDSInputWidget } from "widgets/wds/WDSInputWidget";
import { NumberInputIcon, NumberInputThumbnail } from "appsmith-icons";
import { INPUT_TYPES } from "widgets/wds/WDSInputWidget/constants";
import type { WidgetBaseConfiguration } from "WidgetProvider/constants";
import { INPUT_TYPES } from "widgets/wds/WDSBaseInputWidget/constants";

class WDSNumberInputWidget extends WDSInputWidget {
static type = "WDS_NUMBER_INPUT_WIDGET";
Expand Down
38 changes: 38 additions & 0 deletions app/client/src/widgets/wds/WDSParagraphWidget/__tests__/helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import type { WidgetProps } from "widgets/BaseWidget";
import { fontSizeUpdateHook } from "../helpers";

describe("fontSizeUpdateHook", () => {
it("should update the font size and type", () => {
const props = {
type: "WDS_PARAGRAPH_WIDGET",
} as unknown as WidgetProps;

const updates = fontSizeUpdateHook(props, "fontSize", "heading");

expect(updates).toEqual([
{ propertyPath: "fontSize", propertyValue: "heading" },
{ propertyPath: "type", propertyValue: "WDS_HEADING_WIDGET" },
]);

const updates2 = fontSizeUpdateHook(props, "fontSize", "body");

expect(updates2).toEqual([
{ propertyPath: "fontSize", propertyValue: "body" },
{ propertyPath: "type", propertyValue: "WDS_PARAGRAPH_WIDGET" },
]);

const updates3 = fontSizeUpdateHook(props, "fontSize", "subtitle");

expect(updates3).toEqual([
{ propertyPath: "fontSize", propertyValue: "subtitle" },
{ propertyPath: "type", propertyValue: "WDS_HEADING_WIDGET" },
]);

const updates4 = fontSizeUpdateHook(props, "fontSize", "title");

expect(updates4).toEqual([
{ propertyPath: "fontSize", propertyValue: "title" },
{ propertyPath: "type", propertyValue: "WDS_HEADING_WIDGET" },
]);
});
});
1 change: 1 addition & 0 deletions app/client/src/widgets/wds/WDSParagraphWidget/config/defaultsConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const defaultsConfig = {
fontSize: "body",
textAlign: "left",
textColor: "neutral",
fontStyle: undefined,
Copy link
Contributor Author

@jsartisan jsartisan Sep 3, 2024

Choose a reason for hiding this comment

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

We need to add this, otherwise when hydrating values from session, we won't be able to get this key if not added here. Ideally WidgetFactory.widgetConfigMap.get(type) should return all keys with or without values. But for some reason, we are not getting fontStyle when the key is not added in the defualtsConfig. Noted this to check later.

widgetName: "Paragraph",
shouldTruncate: false,
version: 1,
Expand Down
4 changes: 3 additions & 1 deletion app/client/src/widgets/wds/WDSParagraphWidget/config/propertyPaneConfig/styleConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { TYPOGRAPHY_VARIANTS } from "@appsmith/wds-theming";
import { ValidationTypes } from "constants/WidgetValidation";

import { fontSizeUpdateHook } from "../../helpers";

export const propertyPaneStyleConfig = [
{
sectionName: "General",
Expand Down Expand Up @@ -32,7 +34,7 @@ export const propertyPaneStyleConfig = [
isJSConvertible: true,
isBindProperty: true,
isTriggerProperty: false,
isReusable: true,
Copy link
Contributor Author

@jsartisan jsartisan Sep 3, 2024

Choose a reason for hiding this comment

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

isReusable is not required for fontSize as discussed with taras.

updateHook: fontSizeUpdateHook,
validation: {
type: ValidationTypes.TEXT,
params: {
Expand Down
6 changes: 6 additions & 0 deletions app/client/src/widgets/wds/WDSParagraphWidget/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export const FONT_SIZE_TO_WIDGET_TYPE_MAP = {
body: "WDS_PARAGRAPH_WIDGET",
subtitle: "WDS_HEADING_WIDGET",
title: "WDS_HEADING_WIDGET",
heading: "WDS_HEADING_WIDGET",
} as const;
24 changes: 24 additions & 0 deletions app/client/src/widgets/wds/WDSParagraphWidget/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import type { PropertyUpdates } from "WidgetProvider/constants";
import type { WidgetProps } from "widgets/BaseWidget";

import { FONT_SIZE_TO_WIDGET_TYPE_MAP } from "./constants";

export function fontSizeUpdateHook(
props: WidgetProps,
propertyName: string,
propertyValue: keyof typeof FONT_SIZE_TO_WIDGET_TYPE_MAP,
) {
const updates: PropertyUpdates[] = [
{
propertyPath: propertyName,
propertyValue: propertyValue,
},
];

updates.push({
propertyPath: "type",
propertyValue: FONT_SIZE_TO_WIDGET_TYPE_MAP[propertyValue],
});

return updates;
}
4 changes: 2 additions & 2 deletions app/client/src/widgets/wds/WDSPasswordInputWidget/widget/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { WIDGET_TAGS } from "constants/WidgetConstants";
import { WDSInputWidget } from "widgets/wds/WDSInputWidget";
import { PasswordInputIcon, PasswordInputThumbnail } from "appsmith-icons";
import { INPUT_TYPES } from "widgets/wds/WDSInputWidget/constants";
import { INPUT_TYPES } from "widgets/wds/WDSBaseInputWidget/constants";
import type { WidgetBaseConfiguration } from "WidgetProvider/constants";
import { PasswordInputIcon, PasswordInputThumbnail } from "appsmith-icons";

class WDSPasswordInputWidget extends WDSInputWidget {
static type = "WDS_PASSWORD_INPUT_WIDGET";
Expand Down
1 change: 1 addition & 0 deletions app/client/src/widgets/wds/WDSPhoneInputWidget/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from "./constants";
export { WDSPhoneInputWidget } from "./widget";