appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
33.95k stars 3.66k forks source link

[Bug]: validation and state updates for the inter-dependent controls in the property pane not working #17159

Open ankurrsinghal opened 2 years ago

ankurrsinghal commented 2 years ago

Is there an existing issue for this?

Description

The above bug was found during the development of Dynamic Feature called Auto Height with Limits. Here we have 2 values called min and max limit. Min cannot be greater than max, this is a validation we added. Now the min should check for its validations every time max is updated, which is currently not happening.

More findings:- I added a property update for min control in the update hook for max control. Now after everytime max is updated and its last value was less than min, it sends an update for min control as well from its updateHook. But the error still remains, but as soon as I focus on the min control the red border goes away. After more debugging I found that CodeEditor.tsx which is used in InputTextControl uses a state property called AppState.evaluations.tree, this get updates everytime we change a property of a widget. But I think there is no interdependcy logic built into this, so changing max will change the tree but will not re-render min control.

Steps To Reproduce

  1. Grab a text widget.
  2. Enable the Auto Height with limits.
  3. Set min to be greater than max.
  4. Observe there is an error.
  5. Now update the max to be greater than min.
  6. Observe that the max control error is gone but min control still shows an error.

Please test it herem https://appsmith-mztipjmre-get-appsmith.vercel.app

Public Sample App

https://appsmith-mztipjmre-get-appsmith.vercel.app

Version

Cloud

rishabhrathod01 commented 1 year ago

We need to configure min and max properties for revalidation as mentioned in the PR description https://github.com/appsmithorg/appsmith/pull/17138 to fix above issue.

dhruvikn commented 1 year ago

One more scenario where revalidation is not working, documented here.

cc: @dilippitchika, @somangshu, @satbir121

somangshu commented 1 year ago

@Rishabh-Rathod the issue was linked to the PR, but was re-opened. What happened, the PR does not address this particular issue?

rishabhrathod01 commented 1 year ago

@somangshu the re-validation PR closing this issue was mainly tested with the Tabs widget as the dynamic height feature's PR was still a work in progress. ( not merged to release branch )

After dynamic height and re-validation, PR both were merged in the release branch. We found a few bugs for re-validation while the QA verification process and reopened the above issue to track it.

cc: @satbir121 @AnandiKulkarni

somangshu commented 1 year ago

@Rishabh-Rathod @satbir121 we closed this was a dependency for #17652, which is now merged, Please take this up when you get the chance

AnandiKulkarni commented 1 year ago

Removing QA label as this needs to be picked up -cc @Rishabh-Rathod