feat: Deprecate form button widget #11152

wmdev0808 · 2022-02-14T21:33:32Z

Description

Since we have resetWidget as a trigger function, we can deprecate the form button widget added with every form widget.

Rename all references of FORM_BUTTON_WIDGET to BUTTON_WIDGET
Remove FormButtonWidget from the widgets in Appsmith
Add migrations to change the type of FORM_BUTTON_WIDGET to BUTTON_WIDGET
Migrate the extra features existing in FormButtonWidget to ButtonWidget.

QA Note:

This needs to be a backwards compatible change, i.e, existing Form Widgets must work as they have been so far.
We need to also confirm if FormButtonWidget's features work in the ButtonWidget now. (Reset on click and Disabled on invalid)

Fixes #3613

Type of change

New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Migrations have jest tests
Existing Form Widget tests must pass.

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Test coverage results 🧪

🟢 Total coverage has increased

// Code coverage diff between base branch:release and head branch: feat/3613-deprecate-form-button

Status	File	% Stmts	% Branch	% Funcs	% Lines
🟢	total	55.71 (-0.01)	37.09 (0.04)	35.79 (0)	56.07 (-0.01)
🟢	app/client/src/icons/WidgetIcons.tsx	59.78 (0.64)	100 (0)	15.91 (0.35)	59.78 (0.64)
🟢	app/client/src/sagas/SnipingModeSagas.ts	20 (0)	3.23 (0.1)	40 (0)	20.79 (0)
🟢	app/client/src/utils/DSLMigrations.ts	73.63 (0.36)	70.05 (0.72)	71.72 (0.59)	73.46 (0.38)
🟢	app/client/src/utils/WorkerUtil.ts	89.76 (0.78)	72.55 (1.96)	100 (0)	93.33 (0.95)
🟢	app/client/src/utils/autocomplete/TernServer.ts	52.94 (0.23)	41.67 (0.84)	36.21 (0)	56.99 (0.25)
🔴	app/client/src/widgets/ButtonWidget/widget/index.tsx	61.22 (-9.51)	30 (-12.86)	58.33 (0)	60.42 (-9.58)
❌	~~app/client/src/widgets/FormButtonWidget/index.ts~~	~~100~~	~~100~~	~~100~~	~~100~~
❌	~~app/client/src/widgets/FormButtonWidget/widget/index.tsx~~	~~65.71~~	~~19.05~~	~~62.5~~	~~64.71~~
❌	~~app/client/test/factories/Widgets/FormButtonFactory.ts~~	75	~~100~~	0	75

-- Add disabledWhenInvalid, resetFormOnClick properties to the existing ButtonWidget -- Remove FormButtonWidget and its references -- Adapt FormWidget to use ButtonWidget instead of FormButtonWidget -- Add a migration for converting FORM_BUTTON_WIDGET to BUTTON_WIDGET

-- Add a Jest test case for form button migration

-- Update helpText for disabledWhenInvalid and resetFormOnClick props

vercel · 2022-02-14T21:33:35Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/get-appsmith/appsmith/4uJ7jwY8yfmPw4aEW6UpkMLsfxq8
✅ Preview: https://appsmith-git-feat-3613-deprecate-form-button-get-appsmith.vercel.app

wmdev0808 · 2022-02-14T21:34:26Z

/ok-to-test sha=e133c89

github-actions · 2022-02-14T21:35:00Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1843614680.
Workflow: Appsmith External Integration Test Workflow.
Commit: e133c89.
PR: 11152.

ashit-rath · 2022-02-15T05:06:08Z

app/server/appsmith-server/src/main/resources/system-themes.json

@@ -85,7 +85,7 @@
        "borderRadius": "{{appsmith.theme.borderRadius.appBorderRadius}}",
        "boxShadow": "{{appsmith.theme.boxShadow.appBoxShadow}}"
      },
-      "FORM_BUTTON_WIDGET": {
+      "BUTTON_WIDGET": {


Shouldn't we remove this entry instead of renaming it? We should already have an entry for BUTTON_WIDGET

Please check rest of this file for similar changes and remove if redundant entries are made for BUTTON_WIDGET

-- Eliminate duplicate entries for BUTTON_WIDGET in system-themes.json

ashit-rath · 2022-02-15T10:04:41Z

Note to QA: Please verify the Generate CRUD app. @appsmithorg/qa

wmdev0808 · 2022-02-18T13:23:37Z

/ok-to-test sha=210f8c5

github-actions · 2022-02-18T13:25:05Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1864616102.
Workflow: Appsmith External Integration Test Workflow.
Commit: 210f8c5.
PR: 11152.

github-actions · 2022-02-18T13:53:05Z

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/1864616102.
Commit: 210f8c5.
Results:

Click to view performance test results

	Median	Mean	SD.Sample	SD.Population

-- Change formButtonWidget selector to be for a button widget

wmdev0808 · 2022-02-18T14:35:24Z

/ok-to-test sha=b6c1a03

github-actions · 2022-02-18T15:04:02Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1865005691.
Workflow: Appsmith External Integration Test Workflow.
Commit: b6c1a03.
PR: 11152.

riodeuno · 2022-02-24T10:35:07Z

@appsmithorg/qa This needs extensive testing w.r.t making sure that existing form widgets work exactly as they have so far. Testing this is critical.

-- Change helpTexts for form related properties

wmdev0808 · 2022-02-24T15:10:55Z

/ok-to-test sha=4b768bc

github-actions · 2022-02-24T15:11:57Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1893762268.
Workflow: Appsmith External Integration Test Workflow.
Commit: 4b768bc.
PR: 11152.

github-actions · 2022-02-24T15:46:39Z

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/1893762268.
Commit: 4b768bc.
Results:

Click to view performance test results

	Median	Mean	SD.Sample	SD.Population

wmdev0808 · 2022-03-01T19:48:59Z

/ok-to-test sha=d860aca

github-actions · 2022-03-01T19:49:55Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1918351490.
Workflow: Appsmith External Integration Test Workflow.
Commit: d860aca.
PR: 11152.

github-actions · 2022-03-01T20:17:33Z

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/1918351490.
Commit: d860aca.
Results:

Click to view performance test results

	Median	Mean	SD.Sample	SD.Population

somangshu · 2022-03-04T10:19:37Z

@vivekverma2312 @wmdev0808 A user that has found an issue - formButton.isDisabled is not working. Can we please add it in our test plan for newer alternative? - https://community.appsmith.com/t/how-can-i-validate-if-a-form-is-valid-in-a-widget-outside-the-form/700/2 - I hope its working here.

also @riodeuno should this be fixed in the form button since we are going to maintain it, Or we simply tell the user to migrate to the new form button?

YogeshJayaseelan · 2022-03-04T10:27:10Z

@wmdev0808 @riodeuno @somangshu @vivekverma2312 Testing in progress for this feature and please find the status below,

Issue 1: Disabled Invalid form does not work on passing the JS queries like {{Button3.text==="Submit"}} or {{Input1.text.length>4}}

https://loom.com/share/57d4e3b1b3ae403fa13d99cc52ab4ca1

Questions:

Question 1 - Migration - Opening any older apps which has a form widget - The button name still reads as 'FormButton1' and 'FormButton2' are we okay with keeping the button name as 'FormButton1' or 'FormButton2' in older apps?

Question 2 -Properties of the button is not the same in the table button property

riodeuno · 2022-03-07T07:49:49Z

@YogeshJayaseelan We're not changing the names of the widgets. So, FormButton1, etc, are correct names. The table button properties will be different from the button properties. We can look at standardising this in the widget property pane organisation effort @dilippitchika

@somangshu I was wondering if we have decided to maintain the FormButton widget? The code updates here, deprecates and removes references to the FormButton widget.

If we're removing FormButton widget, @YogeshJayaseelan we need to absolutely make sure that all Form widgets work when migrating to this DP. So, we should work on creating complex forms, and using some of our complex application on release, then opening them in the deploy preview and making sure that they work fine.

If we're still going to maintain FormButton widget, @YogeshJayaseelan we need to make sure freshly added Form widgets in the deploy preview don't have any regression in features.

somangshu · 2022-03-07T10:16:39Z

@riodeuno multiple people suggested that we do not write migration as that will have high chance of breaking an old app, So based on this my assumption would be that we will have to maintain the old formButton

riodeuno · 2022-03-08T12:39:16Z

@somangshu Got it. If we're not migrating then I will suggest we close this PR and create a new one with the limited set of changes. That will only involve updating the Form blueprint and upgrading the button widget's feature set to include the features in the FormButton Widget.

The QA approach for the same should be simply to make sure that any new Form widgets work exactly as they did and have the same features. We might also want to copy the feature's cypress tests from the Form Button widget to the Button Widget.

YogeshJayaseelan · 2022-03-09T08:29:00Z

@riodeuno @somangshu We have paused testing this for now. Will resume testing once the migration-related discussion is closed

CC - @vivekverma2312

somangshu · 2022-03-09T09:04:42Z

@wmdev0808 can you understand the change required here? Please let me know if a discussion is required. Do collect all possible questions before we start our discussion

wmdev0808 · 2022-03-09T10:57:11Z

@wmdev0808 can you understand the change required here? Please let me know if a discussion is required. Do collect all possible questions before we start our discussion

@somangshu , I want your reply with the followings:

Here are my understanding for the new requirements:

We don't need any migration for backward compatibility
The newly created forms will be using ButtonWidget instead of FormButtonWidget, ensuring its original functionalities
ButtonWidget should include from related form fields

Here are my questions:

Am I missing any other requirements?
Does the current implementation has issues with old forms or even newly created forms?
If yes with the above, please add demonstrating videos for my reference.

somangshu · 2022-03-09T11:01:48Z

@wmdev0808 please help me understand what is the migration doing in the PR. I am not sure right now

wmdev0808 · 2022-03-09T11:22:46Z

@wmdev0808 please help me understand what is the migration doing in the PR. I am not sure right now

It just replaces the widget type with button widget's, that is, FORM_BUTTON_WIDGET to BUTTON_WIDGET

somangshu · 2022-03-09T12:00:05Z

It just replaces the widget type with button widget's, that is, FORM_BUTTON_WIDGET to BUTTON_WIDGET

@wmdev0808 what is the reason to do that, @YogeshJayaseelan @vivekverma2312 did we test around this, We need to be sure that this cannot break anything

wmdev0808 · 2022-03-09T12:04:55Z

It just replaces the widget type with button widget's, that is, FORM_BUTTON_WIDGET to BUTTON_WIDGET
@wmdev0808 what is the reason to do that, @YogeshJayaseelan @vivekverma2312 did we test around this, We need to be sure that this cannot break anything

Because we will be using ButtonWidget instead of FormButtonWidget, resulting remove FormButtonWidget in our code base

somangshu · 2022-03-10T07:31:20Z

@riodeuno would this break any old applications

riodeuno · 2022-03-15T07:34:32Z

@somangshu Whether this will break any existing applications is a tough question to answer. Looking at the code and how these widgets work, it seems like they shouldn't. With that said, we need rigorous testing for the migration nevertheless.

This concern is also the reason for contention over whether to

keep maintaining the FORM_BUTTON_WIDGET and use BUTTON_WIDGET as a replacement in new forms or to
replace all references of FORM_BUTTON_WIDGET to BUTTON_WIDGET while migrating existing DSLs as well (the current PR's changes).

Form Widget is one of our most used widgets and breaking changes can impact our users negatively. To avoid a breaking scenario, we are approaching this solution in approach 1.

@wmdev0808 we can consider migration as a separate effort. The idea is to phase this solution into two parts

Stop using Form Button Widget for any new forms added henceforth. This will confirm if the Button widget upgrade is working correctly for our users. Once this is successful,
Deprecate the Form Button Widget and cleanup the codebase with migrations to the widget.

wmdev0808 added 5 commits February 15, 2022 03:41

Merge branch 'release' into feat/3613-deprecate-form-button

0429b0f

feat: Deprecate form button widget

7f3b4e3

-- Add a Jest test case for form button migration

Merge branch 'release' into feat/3613-deprecate-form-button

37a549f

feat: Deprecate form button widget

e133c89

-- Update helpText for disabledWhenInvalid and resetFormOnClick props

wmdev0808 added Enhancement New feature or request Widgets Product This label groups issues related to widgets Button Widget Issues related to the button widget Form Widget labels Feb 14, 2022

wmdev0808 requested review from riodeuno and ashit-rath February 14, 2022 21:33

wmdev0808 self-assigned this Feb 14, 2022

ashit-rath requested changes Feb 15, 2022

View reviewed changes

wmdev0808 added 2 commits February 15, 2022 15:03

feat: Deprecate form button widget

de272a7

-- Eliminate duplicate entries for BUTTON_WIDGET in system-themes.json

Merge branch 'release' into feat/3613-deprecate-form-button

210f8c5

vercel bot deployed to Preview February 15, 2022 07:06 View deployment

wmdev0808 requested a review from ashit-rath February 15, 2022 07:07

ashit-rath previously approved these changes Feb 15, 2022

View reviewed changes

wmdev0808 added 2 commits February 18, 2022 22:30

feat: Deprecate form button widget

a98651e

-- Change formButtonWidget selector to be for a button widget

Merge branch 'release' into feat/3613-deprecate-form-button

b6c1a03

vercel bot deployed to Preview February 18, 2022 14:35 View deployment

wmdev0808 added 2 commits February 24, 2022 23:08

feat: Deprecate form button widget

06d5eee

-- Change helpTexts for form related properties

Merge branch 'release' into feat/3613-deprecate-form-button

4b768bc

vercel bot deployed to Preview February 24, 2022 15:10 View deployment

Merge branch 'release' into feat/3613-deprecate-form-button

d860aca

vercel bot deployed to Preview March 1, 2022 19:47 View deployment

sharat87 dismissed stale reviews from riodeuno and ashit-rath via d860aca March 17, 2022 13:10

somangshu closed this Mar 31, 2022

somangshu mentioned this pull request May 17, 2022

[Feature]: deprecation warning on widgets #12291

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Deprecate form button widget #11152

feat: Deprecate form button widget #11152

wmdev0808 commented Feb 14, 2022 •

edited by github-actions bot

Loading

vercel bot commented Feb 14, 2022 •

edited

Loading

wmdev0808 commented Feb 14, 2022

github-actions bot commented Feb 14, 2022

ashit-rath Feb 15, 2022

ashit-rath Feb 15, 2022

ashit-rath commented Feb 15, 2022 •

edited by riodeuno

Loading

wmdev0808 commented Feb 18, 2022

github-actions bot commented Feb 18, 2022

github-actions bot commented Feb 18, 2022

wmdev0808 commented Feb 18, 2022

github-actions bot commented Feb 18, 2022

riodeuno commented Feb 24, 2022

wmdev0808 commented Feb 24, 2022

github-actions bot commented Feb 24, 2022

github-actions bot commented Feb 24, 2022

wmdev0808 commented Mar 1, 2022

github-actions bot commented Mar 1, 2022

github-actions bot commented Mar 1, 2022

somangshu commented Mar 4, 2022

YogeshJayaseelan commented Mar 4, 2022 •

edited

Loading

riodeuno commented Mar 7, 2022 •

edited

Loading

somangshu commented Mar 7, 2022

riodeuno commented Mar 8, 2022

YogeshJayaseelan commented Mar 9, 2022

somangshu commented Mar 9, 2022

wmdev0808 commented Mar 9, 2022 •

edited

Loading

somangshu commented Mar 9, 2022 •

edited

Loading

wmdev0808 commented Mar 9, 2022 •

edited

Loading

somangshu commented Mar 9, 2022 •

edited

Loading

wmdev0808 commented Mar 9, 2022

somangshu commented Mar 10, 2022

riodeuno commented Mar 15, 2022 •

edited

Loading

feat: Deprecate form button widget #11152

feat: Deprecate form button widget #11152

Conversation

wmdev0808 commented Feb 14, 2022 • edited by github-actions bot Loading

Description

QA Note:

Type of change

How Has This Been Tested?

Checklist:

Test coverage results 🧪

vercel bot commented Feb 14, 2022 • edited Loading

wmdev0808 commented Feb 14, 2022

github-actions bot commented Feb 14, 2022

ashit-rath Feb 15, 2022

Choose a reason for hiding this comment

ashit-rath Feb 15, 2022

Choose a reason for hiding this comment

ashit-rath commented Feb 15, 2022 • edited by riodeuno Loading

wmdev0808 commented Feb 18, 2022

github-actions bot commented Feb 18, 2022

github-actions bot commented Feb 18, 2022

wmdev0808 commented Feb 18, 2022

github-actions bot commented Feb 18, 2022

riodeuno commented Feb 24, 2022

wmdev0808 commented Feb 24, 2022

github-actions bot commented Feb 24, 2022

github-actions bot commented Feb 24, 2022

wmdev0808 commented Mar 1, 2022

github-actions bot commented Mar 1, 2022

github-actions bot commented Mar 1, 2022

somangshu commented Mar 4, 2022

YogeshJayaseelan commented Mar 4, 2022 • edited Loading

riodeuno commented Mar 7, 2022 • edited Loading

somangshu commented Mar 7, 2022

riodeuno commented Mar 8, 2022

YogeshJayaseelan commented Mar 9, 2022

somangshu commented Mar 9, 2022

wmdev0808 commented Mar 9, 2022 • edited Loading

somangshu commented Mar 9, 2022 • edited Loading

wmdev0808 commented Mar 9, 2022 • edited Loading

somangshu commented Mar 9, 2022 • edited Loading

wmdev0808 commented Mar 9, 2022

somangshu commented Mar 10, 2022

riodeuno commented Mar 15, 2022 • edited Loading

wmdev0808 commented Feb 14, 2022 •

edited by github-actions bot

Loading

vercel bot commented Feb 14, 2022 •

edited

Loading

ashit-rath commented Feb 15, 2022 •

edited by riodeuno

Loading

YogeshJayaseelan commented Mar 4, 2022 •

edited

Loading

riodeuno commented Mar 7, 2022 •

edited

Loading

wmdev0808 commented Mar 9, 2022 •

edited

Loading

somangshu commented Mar 9, 2022 •

edited

Loading

wmdev0808 commented Mar 9, 2022 •

edited

Loading

somangshu commented Mar 9, 2022 •

edited

Loading

riodeuno commented Mar 15, 2022 •

edited

Loading