-
Notifications
You must be signed in to change notification settings - Fork 20
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.
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
fix #1391
fix #1391
Conversation
WalkthroughWalkthroughThe changes involve modifications to the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js (3 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/OTPInput.js (3 hunks)
Additional context used
Path-based instructions (2)
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/OTPInput.js (1)
Pattern
**/*.js
: checkmicro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js (1)
Pattern
**/*.js
: check
Biome
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js
[error] 727-727: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
Additional comments not posted (6)
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/OTPInput.js (4)
21-21
: Ensure validation logic is updated to handle non-numeric input.Changing the input type to "text" allows for a broader range of input handling, including the ability to accept empty strings. This change is significant as it alters how user input is processed, enabling the input to accept non-numeric characters.
Please ensure that the validation logic is updated accordingly to handle non-numeric input and prevent any unintended behavior.
32-32
: LGTM!The updated validation logic using the regular expression
/^[0-9]$/
ensures that only valid numeric input (single digit) is accepted. This change aligns with the updated input type and refines the validation process.
63-67
: LGTM!The updated
inputChange
function handles the case where the input is valid (single digit) by updating the OTP value using thechangeCodeAtFocus
function and moving the focus to the next input field using thefocusNextInput
function.This change enhances the user experience by automatically moving the focus to the next input field after a valid input is entered.
67-68
: LGTM!The updated
inputChange
function handles the case where the input is cleared (i.e., when the value is an empty string) by calling thechangeCodeAtFocus
function with an empty string.This change ensures that the application can appropriately manage the state of the input when users delete their entries, enhancing the overall user experience.
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js (2)
762-762
: LGTM!The conditional logic to disable the email field in multi-tenant scenarios looks good and aligns with the provided summary of changes.
779-779
: LGTM!The conditional rendering of the change password button based on the multi-tenancy check looks good and aligns with the provided summary of changes.
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js
Outdated
Show resolved
Hide resolved
…s/citizen/Home/UserProfile.js
…s/citizen/Home/UserProfile.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js (3 hunks)
Additional context used
Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js (1)
Pattern
**/*.js
: check
Biome
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js
[error] 727-727: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
Additional comments not posted (2)
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js (2)
762-762
: LGTM!The change to conditionally disable the email input field based on the result of
Digit.Utils.getMultiRootTenant()
enhances the component's responsiveness to the application's multi-tenant capabilities.
779-779
: LGTM!The change to conditionally render the change password button based on the values of
changepassword
and the result ofDigit.Utils.getOTPBasedLogin()
aligns the component's behavior with the multi-tenant capabilities of the application.
@@ -724,7 +724,7 @@ const UserProfile = ({ stateCode, userType, cityDetails }) => { | |||
name="mobileNumber" | |||
placeholder="Enter a valid Mobile No." | |||
onChange={(value) => setUserMobileNumber(value)} | |||
disable={true} | |||
disable={Digit.Utils.getMultiRootTenant()? false : true} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplify the conditional expression.
The ternary operator is redundant here as the result of Digit.Utils.getMultiRootTenant()
is already a boolean value. Consider simplifying the code by directly assigning the negated result to the disable
prop.
Apply this diff to simplify the code:
-disable={Digit.Utils.getMultiRootTenant()? false : true}
+disable={!Digit.Utils.getMultiRootTenant()}
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
disable={Digit.Utils.getMultiRootTenant()? false : true} | |
disable={!Digit.Utils.getMultiRootTenant()} |
Tools
Biome
[error] 727-727: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
@@ -1,7 +1,11 @@ | |||
import { LocalizationService } from "./Localization/service"; | |||
|
|||
const ADMIN_CODE = ({ tenantId, hierarchyType }) => { | |||
if(Digit.Utils.getMultiRootTenant()){ | |||
return hierarchyType.code; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why overriding this way ? then how the boundary codes gets localised
? @aaradhya-egov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In localisation code, previously it is creating code with tenant but in our case tenant will be dynamic thats why i have removed tenant from localisaed code. from here i am just taking code. and below we have getlocalities function where I am using that localisation code @jagankumar-egov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
No description provided.