Skip to content
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

Jump to bottom

fix: Input, Phone and Currency input widgets valid property validations #10966

Merged
merged 4 commits into from
Feb 10, 2022
Merged

fix: Input, Phone and Currency input widgets valid property validations #10966

merged 4 commits into from
Feb 10, 2022

Conversation

sbalaji1192
Copy link
Contributor

@sbalaji1192 sbalaji1192 commented Feb 8, 2022 •
edited by github-actions bot
Loading

Fixes #10893

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions, so we can reproduce.
Please also list any relevant details for your test configuration.

  • Test A
  • Test B

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: fix/input-widget-validation-property
Status File % Stmts % Branch % Funcs % Lines
🟢 total 55.06 (0) 36.39 (0.01) 34.7 (0) 55.43 (0)
🟢 app/client/src/utils/autocomplete/TernServer.ts 52.94 (0.23) 41.67 (0.84) 36.21 (0) 56.99 (0.25)

@vercel
Copy link

vercel bot commented Feb 8, 2022 •
edited
Loading

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/J1JCJC9TPspR2ckRfgB6ymLr9HSK
✅ Preview: https://appsmith-git-fix-input-widget-validation-property-get-appsmith.vercel.app

@github-actions github-actions bot added the Bug Something isn't working label Feb 8, 2022
@sbalaji1192
Copy link
Contributor Author

sbalaji1192 commented Feb 8, 2022 •
edited
Loading

/ok-to-test sha=3377ab8

@sbalaji1192
Copy link
Contributor Author

/ok-to-test sha=3377ab8

@github-actions
Copy link

github-actions bot commented Feb 9, 2022

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1816044832.
Workflow: Appsmith External Integration Test Workflow.
Commit: 3377ab8.
PR: 10966.

@github-actions
Copy link

github-actions bot commented Feb 9, 2022

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/1816044832.
Commit: 3377ab8.
Results:

Click to view performance test results

Run 1 Run 2 Run 3 Run 4 Run 5 Median Mean SD.Sample SD.Population
Click on table row
scripting 2505.96 2297.03 2342.2 2607.52 2523.8 2505.96 2455.3 5.32 4.76
painting 22.19 13.64 28.45 18.68 19.06 19.06 20.4 26.67 23.87
rendering 384.46 339.72 342.6 399.63 334.34 342.6 360.15 8.26 7.39
Edit input
scripting 401.12 372.21 311.13 459.43 359.03 372.21 380.58 14.39 12.87
painting 16.11 18.83 15.07 6.99 4.81 15.07 12.36 49.43 44.17
rendering 19.17 17.69 12.84 14.53 25.92 17.69 18.03 28.12 25.18
Clear input
scripting 1096.71 785.85 770.26 881.79 765.78 785.85 860.08 16.33 14.61
painting 7.06 7.23 6.49 5.57 13.43 7.06 7.96 39.32 35.18
rendering 235.55 221.85 222.91 242.89 229.83 229.83 230.61 3.83 3.43
Edit input again
scripting 301.33 385.02 317.2 328.55 406.44 328.55 347.71 13.09 11.71
painting 6.58 5.77 7.35 6.23 8.23 6.58 6.83 14.20 12.74
rendering 19.08 21.31 21.65 28.73 23.93 21.65 22.94 15.95 14.30

@YogeshJayaseelan
Copy link
Contributor

@sbalaji1192 Tested this PR and it is working as expected

@sbalaji1192
fix: empty string in Input validation property evaulates to false
56f0bfb 
- When user removed an expression from the validation property of
  input widget, its value becomes `""`.And in boolean validation
  function we treat this empty string as false. so Input widget
  ends up with isValid false and we show error border around the
  Input. so we are setting a default value true, so when there is
  no expression it evaulates to true.
@sbalaji1192
cypress test
3685b0c
@sbalaji1192
cypress test case message
b1dfe8e
@sbalaji1192
typo
9ee2155
@sbalaji1192 sbalaji1192 force-pushed the fix/input-widget-validation-property branch from 3377ab8 to 9ee2155 Compare February 10, 2022 08:44
@sbalaji1192
Copy link
Contributor Author

/ok-to-test sha=9ee2155

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1822753854.
Workflow: Appsmith External Integration Test Workflow.
Commit: 9ee2155.
PR: 10966.

@sbalaji1192 sbalaji1192 merged commit e67474a into release Feb 10, 2022
@sbalaji1192 sbalaji1192 deleted the fix/input-widget-validation-property branch February 10, 2022 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashit-rath ashit-rath ashit-rath approved these changes

Assignees
No one assigned
Labels
Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: Currency Input stops accepting valid input after removing condition in valid property
3 participants
@sbalaji1192 @YogeshJayaseelan @ashit-rath