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: numeric type input widget should accept zero after decimal point #11923

Merged
merged 4 commits into from
Mar 18, 2022
Merged

fix: numeric type input widget should accept zero after decimal point #11923

merged 4 commits into from
Mar 18, 2022

Conversation

sbalaji1192
Copy link
Contributor

@sbalaji1192 sbalaji1192 commented Mar 17, 2022 •
edited by github-actions bot
Loading

Description

Originally we solved this issue by converting the text property into a derived property. but that started causing issues when the Input widget was placed inside the List widget. #11833
Note:

  • Ideally, the text property of the input widget should be a derived property.
    But derived properties of List widget children won't work. so having the text
    property of the Input widget as a derived property causes issues. hence changing
    it to a meta property.

Fixes #11449
Fixes #11839

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 on 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-decimal-value
Status File % Stmts % Branch % Funcs % Lines
🟢 total 55.59 (0.02) 36.87 (0.04) 34.87 (0.01) 55.9 (0.03)
🟢 app/client/src/selectors/commentsSelectors.ts 85.25 (1.64) 64.71 (2.95) 73.33 (0) 90.59 (2.35)
🟢 app/client/src/widgets/BaseInputWidget/widget/index.tsx 67.5 (2.5) 7.14 (0.89) 61.54 (-5.13) 71.05 (2.63)
✨ 🆕 app/client/src/widgets/InputWidgetV2/widget/Utilities.ts 92.31 87.5 100 92.31
🟢 app/client/src/widgets/InputWidgetV2/widget/index.tsx 81.3 (7.7) 68.18 (7.31) 78.95 (-2.3) 80.83 (7.88)
🔴 app/client/src/widgets/PhoneInputWidget/widget/index.tsx 39.39 (-1.24) 6.52 (0) 40 (-2.86) 39.18 (-1.25)

@sbalaji1192
fix: numeric type input widget should accept zero after decimal point (…
9b534d1 
…#11579)

- Derived properties of List widget children won't work. so having the text
  property of Input wiget as a derived property causes issue. hence
  changing it to a meta property.
@vercel
Copy link

vercel bot commented Mar 17, 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/FKxnyB1irQc1UoX7Su9PMw6QWHqS
✅ Preview: https://appsmith-git-fix-input-widget-decimal-value-get-appsmith.vercel.app

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

/ok-to-test sha=9b534d1

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1996204796.
Workflow: Appsmith External Integration Test Workflow.
Commit: 9b534d1.
PR: 11923.

@sbalaji1192
Copy link
Contributor Author

/ok-to-test sha=4be3ce4

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1996507675.
Workflow: Appsmith External Integration Test Workflow.
Commit: 4be3ce4.
PR: 11923.

jsartisan
jsartisan previously approved these changes Mar 17, 2022
View reviewed changes
@vivekverma2312
Copy link

@sbalaji1192 Tested and LGTM

@sbalaji1192
Copy link
Contributor Author

/ok-to-test sha=4be3ce4

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1998348612.
Workflow: Appsmith External Integration Test Workflow.
Commit: 4be3ce4.
PR: 11923.

@sbalaji1192
Copy link
Contributor Author

/ok-to-test sha=4be3ce4

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1999728037.
Workflow: Appsmith External Integration Test Workflow.
Commit: 4be3ce4.
PR: 11923.

@sbalaji1192
Copy link
Contributor Author

/ok-to-test sha=20c699b

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1999837660.
Workflow: Appsmith External Integration Test Workflow.
Commit: 20c699b.
PR: 11923.

@sbalaji1192
Copy link
Contributor Author

/ok-to-test sha=ad3a3ea

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2002523033.
Workflow: Appsmith External Integration Test Workflow.
Commit: ad3a3ea.
PR: 11923.

@github-actions
Copy link

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

Click to view performance test results

Median Mean SD.Sample SD.Population

@sbalaji1192 sbalaji1192 merged commit f1d1ee9 into release Mar 18, 2022
@sbalaji1192 sbalaji1192 deleted the fix/input-widget-decimal-value branch March 18, 2022 16:48
rishabhrathod01 added a commit that referenced this pull request Apr 30, 2022
@rishabhrathod01
Revert "fix: numeric type input widget should accept zero after decim…
855f43e 
…al point (#11923)"

This reverts commit f1d1ee9.
agawda pushed a commit to TalentAlpha/appsmith that referenced this pull request Jan 20, 2023
@sbalaji1192 @agawda-talent-alpha
fix: numeric type input widget should accept zero after decimal point (…
d2186ec 
…appsmithorg#11923)

- Derived properties of List widget children won't work. so having the text
  property of the Input widget as a derived property causes issues. hence
  having it as a meta property.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsartisan jsartisan jsartisan approved these changes

Assignees
No one assigned
Labels
Bug Something isn't working
Projects
None yet
Milestone
No milestone
3 participants
@sbalaji1192 @vivekverma2312 @jsartisan