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
34.79k stars 3.77k forks source link

[Feature] [Enhancement] : Input widget v2 with improved validations #9119

Closed sbalaji1192 closed 2 years ago

sbalaji1192 commented 3 years ago

Is there an existing issue for this?

Summary

Current input widget has lot of open issues related to validation. Fixing them will break the existing application. So let's introduce a version 2 of input widget with improved validations.

Existing issues with input widget validation.

Goal of this issue is to fix the existing issues.

Why should this be worked on?

Input widget is one of the most used widget. Improved input widget will definitely help user in creating solid applications.

sbalaji1192 commented 2 years ago

@vivekverma2312 Could you test this on DP - https://deploy-preview-9582--appsmith-editor-ce.netlify.app/applications?

vivekverma2312 commented 2 years ago

@sbalaji1192 Few issues observed during the DP testing

  1. Auto suggestion is not working when I am trying to bind input widget in text widget
  2. In case of numbers type when I delete the data from field then 0 populates and I am not able to delete that. LOOM DEMO
  3. Suggestion: Instead of writing {{typeof Input1.text}} can we make it {{Input1.text.type}}
  4. Allow currency change is disabled by default and Allow country code change enabled by default, can we make it consistent
  5. Reset on submit is not functional for currency input

Testing is still in progress

aswathkk commented 2 years ago

@sbalaji1192 Can we include #9822 in this?

sbalaji1192 commented 2 years ago

@aswathkk will try to squeeze this in.

vivekverma2312 commented 2 years ago

@sbalaji1192 #7899, #7810, #8012 still exist. In #7675 now NaN is observed on deleting the value LOOM DEMO

sbalaji1192 commented 2 years ago

@vivekverma2312 Could you tell me how did you test #7899 and #8012 ?

sbalaji1192 commented 2 years ago

@vivekverma2312 please test this on - https://appsmith-git-fix-improved-input-widget-validation-get-appsmith.vercel.app/applications/61976b5f86af5222e17018e0/pages/61976b5f86af5222e17018e2/edit

vivekverma2312 commented 2 years ago

@sbalaji1192 Observed new issue after fix for issue2

  1. Default data populated after deleting the value in Input widget LOOM DEMO

    7810 and #8012 still exist

sbalaji1192 commented 2 years ago

@vivekverma2312 fixed #6. Regarding #7810, It seems like an issue with evaluations not with input widget itself. Will further discuss about this with @somangshu . Regarding #8012, It is working as expected. The app provided by the user has work around in validations logic, so it is failing with input widget v2. you need to edit the logics and use appropriate dataype to make it work.

somangshu commented 2 years ago

@sbalaji1192 regarding #8012, does it mean we need to think about any kind of a migration. It would be unacceptable for the users to have their app broken without knowledge, Usually we solve for this. Help me understand more here.

sbalaji1192 commented 2 years ago

@somangshu So the user who created 8012 has created an app with input widget v1 to demonstrate the issues with validations. where he wrote conditions with all input types values as datatype string, which is not the case with input widget v2. based on the input chosen, the datatype of values changes. if we use the same conditions from that app. it will fail because of mismatching datatype. we don't need any migrations for this change.

somangshu commented 2 years ago

I understood, The condition any ways need to be re-written on the input v2, we are good to go then

vivekverma2312 commented 2 years ago

@sbalaji1192 Observed one crash, If we drag and drop currency input on new page then widget crashes LOOM DEMO

YogeshJayaseelan commented 2 years ago

@sbalaji1192 @vivekverma2312 Selected country code/flag is not displayed in the widget and it is observed that only after refreshing the page populates the selected country code/Flag in the widget

https://loom.com/share/918996f3a9bb482487d68b3077ce0b92

YogeshJayaseelan commented 2 years ago

@sbalaji1192 The watermark text of tool tip field in both phone and currency widget reads about 'Password' - Can we change this with relevant watermark text

Screenshot 2021-12-31 at 11 23 00 AM
sbalaji1192 commented 2 years ago

@YogeshJayaseelan @vivekverma2312 Fixed your callouts. please test again.

vivekverma2312 commented 2 years ago

@sbalaji1192 When I am trying to bind the Input widget to text widget I am getting auto suggestion for country code and currency country code

Screenshot 2022-01-05 at 10 55 09 AM
sbalaji1192 commented 2 years ago

@vivekverma2312 Fixed. please test again.

vivekverma2312 commented 2 years ago

@sbalaji1192 1. County code is not updating for United States for Phone Input widget LOOM DEMO

  1. Country flag is not coming on country code drop-down options for Phone Input widget
sbalaji1192 commented 2 years ago

@vivekverma2312 Fixed your callouts. Also fixed callouts posted by @dilippitchika over here. Please verify both.

YogeshJayaseelan commented 2 years ago

@sbalaji1192 Tested this PR and please find the status below,

Issue raised by @vivekverma2312 are fixed and with respect to issues raised by @dilippitchika there is one bug reopen with the below steps

Currency widget - Add 2 decimals to default text keeping the decimal point option as 0 and check for the error message and now change the decimal point option 2 and see the error message will still displayed

LOOM DEMO

sbalaji1192 commented 2 years ago

@YogeshJayaseelan It's an issue on the property pane, not with the widget itself. if the dropdown value is 0, the property pane is not updating the value. Could you create a separate issue for this ?

shwetha-ramesh commented 2 years ago

@sbalaji1192 tested this and found below issues.

  1. When PhoneInput and Currencyinput widget are resized the border moves inside the widget. This does not happen for Input widget image https://loom.com/share/347a1c72c1b84be5a6b9bfa72899621b

  2. When PhoneInput and Currencyinput widget are resized user is able to place Input widget on top of it. image https://loom.com/share/4a88834d835d416888bf9e0d1365d16d

  3. There is no icon present in Entity explorer for input widget image

  4. Icons disappear when Icon alignment is set to right for Number and Password Data type in Input widget https://loom.com/share/4ccaeb50120248ec947e93d7d19b4db2

  5. Tooltip watermark for any Data type selected in Input widget is not relevant. image

  6. Background color for Phoneinput and Currencyinput varies whenAllow country code change and Allow currency change are enabled. image

  7. Property pane in Input widget shows same Error message watermark for all Data types. image

  8. No error message when negative value is entered in Max Chars for Input widget. image

sbalaji1192 commented 2 years ago

@shwetha-ramesh WRT #1 and #2, We have country dropdown and currency dropdown associated with the widget so we can't reduce the width beyond the length of the dropdown. And Also it seems like an invalid scenario.

4, Password show/hide icon and increment/decrement controls are occupying the right-hand side, so we can't place the icon over there - @somangshu need your input on this.

5, We cannot change the placeholder text based on the input data type. so using a common sentence that applies to all types.

Fixed rest. please test.

somangshu commented 2 years ago

@sbalaji1192 here is my thought, in case of the number input type we can add a margin right to the icon such that it is not overlapped, that is in case this is the problem.

YogeshJayaseelan commented 2 years ago

@sbalaji1192 verified the fixed issues and they are working as expected

btsgh commented 2 years ago

All of the existing issues in the bug description, which this bug is supposed to address, have been verified and closed. Hence marking this as Verified as well.