Draft
Conversation
… wrapper for password errors
Contributor
WalkthroughDeprecated the legacy PasswordCombo (removed docs mapping and hidden in toolbox). Added password support to textField: complexity settings, runtime validators, truncated tooltip error UI, hidden min/max in settings, styles, and a migrator converting passwordCombo into a password textField plus confirm field. Changes
Sequence DiagramsequenceDiagram
participant Designer as Form Designer
participant TextField as textField (password)
participant Settings as Settings Store
participant Validator as Validation Engine
participant UI as Tooltip/Error Display
Designer->>TextField: Configure field with textType: 'password'
TextField->>Settings: usePasswordComplexitySettings()
Settings-->>TextField: Complexity config
Designer->>TextField: onChange(value)
TextField->>Validator: validatePasswordValue(value, settings)
Validator-->>TextField: errors[] or []
TextField->>UI: Update passwordError state
UI-->>Designer: Show tooltip/truncated error (if errors)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shesha-reactjs/src/designer-components/passwordCombo/index.tsx`:
- Around line 41-44: The catch block in the password validator (inside
passwordCombo component where you call callback on error) assumes the caught
variable e has a .message property which is invalid under TypeScript strict
mode; update the catch to narrow e safely (e.g., use if (e instanceof Error) {
msg = e.message } else { msg = String(e) }) and pass that msg to callback and
console.error so you never access .message on an unknown type and preserve
meaningful error text.
In `@shesha-reactjs/src/designer-components/textField/utils.ts`:
- Around line 103-106: The catch block in the password validator uses the
unknown catch variable e and accesses e.message directly; change the handler to
narrow e (e.g., treat it as unknown and derive a message via e instanceof Error
? e.message : String(e)) and use that sanitized message in both
console.error('[TextField] Password validator error:', ...) and
callback('Password validation failed: ' + message). Update the catch signature
(catch (e)) handling in the TextField password validation utility so you never
access e.message without narrowing and you pass a safe string to callback and
console.error.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1b539991-70c0-41be-a1f0-025aa6c83bd3
⛔ Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.jsonand included by noneshesha-functional-tests/backend/src/Boxfusion.SheshaFunctionalTests.Web.Host/appsettings.jsonis excluded by!shesha-functional-tests/**and included by noneshesha-reactjs/eslint.config.mjsis excluded by none and included by noneshesha-reactjs/package-lock.jsonis excluded by!**/package-lock.jsonand included by none
📒 Files selected for processing (7)
shesha-reactjs/src/components/componentErrors/component-docs.jsonshesha-reactjs/src/designer-components/passwordCombo/index.tsxshesha-reactjs/src/designer-components/textField/settingsForm.tsshesha-reactjs/src/designer-components/textField/styles.tsshesha-reactjs/src/designer-components/textField/textField.tsxshesha-reactjs/src/designer-components/textField/utils.tsshesha-reactjs/src/providers/form/defaults/toolboxComponents.ts
💤 Files with no reviewable changes (1)
- shesha-reactjs/src/components/componentErrors/component-docs.json
Contributor
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 `@shesha-reactjs/src/designer-components/textField/textField.tsx`:
- Line 97: Replace the parameter type for the onChangeInternal function from
any[] to unknown[] and then narrow or assert the first argument before accessing
currentTarget.value: in onChangeInternal inspect/type-guard the args[0] (e.g.,
check it's an object with a currentTarget property or use a type assertion to
the expected event type) so the expression args[0]?.currentTarget?.value is
type-safe; update the handler logic inside onChangeInternal accordingly to avoid
any usage while preserving existing behavior.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c54ae95c-d302-4c29-bbac-aba14256e99b
📒 Files selected for processing (3)
shesha-reactjs/src/designer-components/passwordCombo/index.tsxshesha-reactjs/src/designer-components/textField/textField.tsxshesha-reactjs/src/designer-components/textField/utils.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shesha-reactjs/src/designer-components/textField/textField.tsx (1)
46-54: 🧹 Nitpick | 🔵 TrivialConsider narrowing the dependency array.
The
useMemoforregExpObjonly usesmodel.regExpandmodel.propertyName, but the dependency array includes the entiremodelobject. This causes unnecessary recalculation when unrelated model properties change.♻️ Proposed optimization
const regExpObj = useMemo(() => { if (!model.regExp) return null; try { return new RegExp(model.regExp, 'g'); } catch (error) { console.warn(`Invalid regExp pattern for '${model.propertyName}':`, model, error); return null; } - }, [model]); + }, [model.regExp, model.propertyName]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shesha-reactjs/src/designer-components/textField/textField.tsx` around lines 46 - 54, The useMemo creating regExpObj depends only on model.regExp and model.propertyName but currently lists the whole model in its dependency array, causing unnecessary recalcs; change the dependency array on the useMemo that returns regExpObj to use [model.regExp, model.propertyName] (keeping the same try/catch and console.warn behavior) so the memo only updates when the pattern or propertyName actually change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@shesha-reactjs/src/designer-components/textField/textField.tsx`:
- Around line 46-54: The useMemo creating regExpObj depends only on model.regExp
and model.propertyName but currently lists the whole model in its dependency
array, causing unnecessary recalcs; change the dependency array on the useMemo
that returns regExpObj to use [model.regExp, model.propertyName] (keeping the
same try/catch and console.warn behavior) so the memo only updates when the
pattern or propertyName actually change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 29a1dfb1-c84c-4c28-b4c2-3ad33c3e178a
📒 Files selected for processing (1)
shesha-reactjs/src/designer-components/textField/textField.tsx
Contributor
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 `@shesha-reactjs/src/designer-components/textField/textField.tsx`:
- Line 58: The tooltip and passwordError state drift because password complexity
is evaluated independently in onChangeInternal instead of using the effective
validator that the form actually commits; compose the password validator into
model.validate.validator (or into the effective validator used by the component)
so there is a single source of truth, then derive the tooltip message by running
that composed validator against the committed value (or by computing
passwordError from validate(value)) inside a useEffect that listens to
value/model.validate changes rather than only in onChangeInternal; remove or
stop updating the separate passwordError state ad-hoc and ensure components like
passwordError, setPasswordError, onChangeInternal, model.validate?.validator and
the tooltip rendering all read from the composed validator result.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3d36919c-3014-45d2-9f75-afe9be13ae57
📒 Files selected for processing (1)
shesha-reactjs/src/designer-components/textField/textField.tsx
|
|
||
| const isPassword = model.textType === 'password'; | ||
| const passwordComplexity = usePasswordComplexitySettings(); | ||
| const [passwordError, setPasswordError] = useState<string | null>(null); |
Contributor
There was a problem hiding this comment.
Use one source of truth for password validation.
Line 66 skips injecting the password validator when model.validate?.validator already exists, but Lines 106-110 still compute passwordError from the complexity rules, and Lines 134-139 surface that state in the tooltip. This can make the tooltip warn about a rule the form is not actually enforcing, and because that state is only updated inside onChangeInternal, it can also drift after resets or other external value updates. Please drive the tooltip from the committed value and the effective validator set, or compose the password validator into the existing validator before both paths use it.
Also applies to: 65-76, 97-110, 134-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shesha-reactjs/src/designer-components/textField/textField.tsx` at line 58,
The tooltip and passwordError state drift because password complexity is
evaluated independently in onChangeInternal instead of using the effective
validator that the form actually commits; compose the password validator into
model.validate.validator (or into the effective validator used by the component)
so there is a single source of truth, then derive the tooltip message by running
that composed validator against the committed value (or by computing
passwordError from validate(value)) inside a useEffect that listens to
value/model.validate changes rather than only in onChangeInternal; remove or
stop updating the separate passwordError state ad-hoc and ensure components like
passwordError, setPasswordError, onChangeInternal, model.validate?.validator and
the tooltip rendering all read from the composed validator result.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#4527
Summary by CodeRabbit
New Features
Migration
Deprecations
Bug Fixes