fix(frontend): trigger numeric keypad for Currency Input on iOS#41731
fix(frontend): trigger numeric keypad for Currency Input on iOS#41731Arbab1308 wants to merge 16 commits intoappsmithorg:releasefrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds HTML5 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Arbab1308
changed the title
|
/ok-to-test |
/ok-to-test |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings used |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/client/src/widgets/BaseInputWidget/component/index.tsx (1)
503-523: UseInputModeenum ingetInputModeto prevent string drift.Line 508 can return
InputMode | undefinedand map via enum members instead of raw strings.Proposed refactor
-import { InputTypes } from "../constants"; +import { InputMode, InputTypes } from "../constants"; ... - getInputMode(inputType: InputHTMLType = "TEXT"): string | undefined { + getInputMode(inputType: InputHTMLType = "TEXT"): InputMode | undefined { switch (inputType) { case "NUMBER": - return "decimal"; + return InputMode.DECIMAL; case "TEL": - return "tel"; + return InputMode.TEL; case "EMAIL": - return "email"; + return InputMode.EMAIL; default: return undefined; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/widgets/BaseInputWidget/component/index.tsx` around lines 503 - 523, The getInputMode function currently returns raw strings; change its signature to return InputMode | undefined (instead of string | undefined) and map InputHTMLType cases to the corresponding InputMode enum members (e.g., return InputMode.Decimal for "NUMBER", InputMode.Tel for "TEL", InputMode.Email for "EMAIL"), leaving default as undefined; update any imports/usages to reference the InputMode enum and ensure InputHTMLType is still the switch discriminant in the getInputMode method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/client/src/widgets/BaseInputWidget/component/index.tsx`:
- Around line 503-523: The getInputMode function currently returns raw strings;
change its signature to return InputMode | undefined (instead of string |
undefined) and map InputHTMLType cases to the corresponding InputMode enum
members (e.g., return InputMode.Decimal for "NUMBER", InputMode.Tel for "TEL",
InputMode.Email for "EMAIL"), leaving default as undefined; update any
imports/usages to reference the InputMode enum and ensure InputHTMLType is still
the switch discriminant in the getInputMode method.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 27969c03-5685-4697-ac0a-c8a702d705fc
📒 Files selected for processing (3)
app/client/src/widgets/BaseInputWidget/component/index.tsxapp/client/src/widgets/BaseInputWidget/constants.tsapp/client/src/widgets/CurrencyInputWidget/component/index.tsx
|
Hi maintainers, could you please approve the pending workflows for this fork PR? Thanks! |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
sebastianiv21
left a comment
sebastianiv21
left a comment
There was a problem hiding this comment.
Thanks for this fix — the core approach is correct and the InputMode enum is a good infrastructure addition. A couple of things before this is ready to merge:
- No unit tests — getInputMode() is a pure mapping function and is straightforward to test. Please add at minimum a test covering NUMBER → "decimal", TEL → "tel", EMAIL → "email", and the default → undefined path.
- Branch is out-of-date — please rebase onto release before requesting a re-review.
Arbab1308
force-pushed
the
fix/currency-inputmode-clean
branch
from
45039fd to
39310aa
Compare
Arbab1308
left a comment
Arbab1308
left a comment
There was a problem hiding this comment.
i have pushed these check once again please
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/client/src/widgets/BaseInputWidget/component/_tests_/getInputMode.test.ts`:
- Line 2: The test imports getInputMode as a standalone function but
getInputMode is a method on BaseInputComponent and not exported; fix by either
extracting getInputMode into a standalone exported function (e.g., create a
utility function getInputMode in the same module and export it from index) and
update callers/tests to import it, or update the test to instantiate
BaseInputComponent and call instance.getInputMode; locate the method on the
BaseInputComponent class and either move its logic into a new exported function
or change the test to construct BaseInputComponent before calling getInputMode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e8ecb7fc-01cb-4adb-b4bc-3febc686c8b7
📒 Files selected for processing (4)
app/client/src/widgets/BaseInputWidget/component/_tests_/getInputMode.test.tsapp/client/src/widgets/BaseInputWidget/component/index.tsxapp/client/src/widgets/BaseInputWidget/constants.tsapp/client/src/widgets/CurrencyInputWidget/component/index.tsx
✅ Files skipped from review due to trivial changes (2)
- app/client/src/widgets/CurrencyInputWidget/component/index.tsx
- app/client/src/widgets/BaseInputWidget/constants.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/widgets/BaseInputWidget/component/index.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/client/src/widgets/BaseInputWidget/component/__tests__/getInputMode.test.ts (1)
17-19: Consider testing the default-parameter path explicitlyOptional: add
expect(getInputMode()).toBeUndefined()to lock the no-arg/default behavior in addition to the"TEXT"case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/widgets/BaseInputWidget/component/__tests__/getInputMode.test.ts` around lines 17 - 19, Test coverage misses the no-argument/default path for getInputMode — add an assertion that calling getInputMode() without arguments returns undefined. Modify the test in getInputMode.test.ts (the existing "returns undefined for default/unknown type" case) to include expect(getInputMode()).toBeUndefined() alongside the existing expect(getInputMode("TEXT")).toBeUndefined() so the default/no-arg behavior is locked in.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/client/src/widgets/BaseInputWidget/component/__tests__/getInputMode.test.ts`:
- Line 2: The test importing getInputMode as a named export is failing because
getInputMode is a class method on BaseInputWidget rather than a standalone
export; either export a standalone function getInputMode from index.tsx (move or
wrap BaseInputWidget.prototype.getInputMode into an exported function) and
update the test import, or change the test to import BaseInputWidget,
instantiate (or use a test double) and call its getInputMode method; reference
the getInputMode method and BaseInputWidget class when making the change so the
import and call are consistent.
---
Nitpick comments:
In
`@app/client/src/widgets/BaseInputWidget/component/__tests__/getInputMode.test.ts`:
- Around line 17-19: Test coverage misses the no-argument/default path for
getInputMode — add an assertion that calling getInputMode() without arguments
returns undefined. Modify the test in getInputMode.test.ts (the existing
"returns undefined for default/unknown type" case) to include
expect(getInputMode()).toBeUndefined() alongside the existing
expect(getInputMode("TEXT")).toBeUndefined() so the default/no-arg behavior is
locked in.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1b3104f9-1f99-4d20-b74a-c3454e7a2641
📒 Files selected for processing (1)
app/client/src/widgets/BaseInputWidget/component/__tests__/getInputMode.test.ts
|
@Arbab1308 please fix the quality check errors |
sebastianiv21
left a comment
sebastianiv21
left a comment
There was a problem hiding this comment.
There are still linting issues
i have tried to fix it again can you please check again |
@Arbab1308 please check the new linting errors |
2 participants
Description
This PR improves mobile data entry by setting
inputMode="decimal"for the Currency Input widget, so iOS/Android show the numeric keypad instead of the full keyboard.What changed
Testing
Fixes
Automation
/ok-to-test tags="@tag.All"
Summary by CodeRabbit
New Features
Tests
Style