appsmith icon indicating copy to clipboard operation
appsmith copied to clipboard

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

Open ankurrsinghal opened this issue 2 years ago • 6 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

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

ankurrsinghal avatar Sep 28 '22 15:09 ankurrsinghal

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.

rishabhrathod01 avatar Oct 26 '22 05:10 rishabhrathod01

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

cc: @dilippitchika, @somangshu, @satbir121

dhruvikn avatar Nov 03 '22 13:11 dhruvikn

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

somangshu avatar Nov 16 '22 08:11 somangshu

@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

rishabhrathod01 avatar Nov 17 '22 04:11 rishabhrathod01

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

somangshu avatar Dec 02 '22 07:12 somangshu

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

AnandiKulkarni avatar May 08 '23 07:05 AnandiKulkarni