fix: disable manage validators if the account is already a validator #1634
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/extension-polkagate/src/fullscreen/stake/solo/partials/CommonTasks.tsx (1)
83-84: Consider simplifying the conditional logicThe bounce and color conditions share the same logic (
!!stakedButNoValidators && !isValidator). Consider extracting this to a variable for better maintainability.+ const shouldHighlight = !!stakedButNoValidators && !isValidator; <FontAwesomeIcon - bounce={!!stakedButNoValidators && !isValidator} - color={stakedButNoValidators && !isValidator ? `${theme.palette.warning.main}` : `${iconColor}`} + bounce={shouldHighlight} + color={shouldHighlight ? `${theme.palette.warning.main}` : `${iconColor}`} fontSize='22px' icon={faHand} />packages/extension-polkagate/src/fullscreen/stake/solo/partials/ActiveValidators.tsx (1)
Line range hint
108-121: Consider refining the loading state logic.The current condition
activeValidators === undefined || isValidator === undefinedmight show loading skeletons in cases where only one of these values is undefined. Consider handling these states separately for more precise user feedback.Here's a suggested improvement:
- : activeValidators === undefined || isValidator === undefined + : activeValidators === undefined ? <Grid container> <Skeleton animation='wave' height='20px' sx={{ display: 'inline-block', fontWeight: 'bold', my: '5px', transform: 'none', width: '100%' }} /> </Grid> + : isValidator === undefined + ? <Typography color='text.primary' fontSize='16px' fontWeight={400}> + {t('Checking validator status...')} + </Typography> : isValidator
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/extension-polkagate/src/fullscreen/stake/solo/StakedSolo.tsx(3 hunks)packages/extension-polkagate/src/fullscreen/stake/solo/partials/ActiveValidators.tsx(1 hunks)packages/extension-polkagate/src/fullscreen/stake/solo/partials/CommonTasks.tsx(2 hunks)
🔇 Additional comments (9)
packages/extension-polkagate/src/fullscreen/stake/solo/partials/CommonTasks.tsx (3)
22-22: LGTM: Well-typed prop addition
The isValidator prop type appropriately handles all possible states (true/false/null/undefined) for validator status.
25-25: LGTM: Component signature properly updated
The function signature correctly includes the new isValidator prop while maintaining the existing structure.
80-80: LGTM: Validator management correctly disabled
The disabled condition properly implements the PR objective by preventing validator management for existing validators while maintaining the original staking-related checks.
packages/extension-polkagate/src/fullscreen/stake/solo/partials/ActiveValidators.tsx (3)
16-16: LGTM! Clean interface update.
The removal of the useIsValidator hook and addition of the isValidator prop aligns well with the component's new responsibility model. The type definition properly handles all possible states.
Also applies to: 21-22
25-25: LGTM! Clean function signature update.
The component signature has been properly updated to match the Props interface while maintaining good TypeScript practices.
Line range hint 1-143: Verify consistent usage of isValidator prop.
The implementation successfully handles validator status through props, but let's verify that all parent components are passing the prop correctly.
✅ Verification successful
Props usage is consistent and properly implemented
The verification shows that:
ActiveValidatorscomponent is used inStakedSolo.tsxwith theisValidatorprop correctly passed- The
isValidatorvalue comes from a dedicated hookuseIsValidatorwhich properly handles all states (boolean | null | undefined) - The implementation matches the component's prop interface and handling logic
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of isValidator prop in parent components
# Search for components that render ActiveValidators
rg -l "ActiveValidators" --type tsx
# Check the prop usage in those files
rg -A 5 "<ActiveValidators" --type tsx
Length of output: 132
Script:
#!/bin/bash
# Let's try without file type filtering since tsx extension is not recognized
# First find files containing ActiveValidators
rg -l "ActiveValidators"
# Then check usage patterns
rg -A 5 "<ActiveValidators"
Length of output: 901
Script:
#!/bin/bash
# Let's check how isValidator is determined in the parent component
rg -B 10 -A 5 "isValidator.*=.*" packages/extension-polkagate/src/fullscreen/stake/solo/StakedSolo.tsx
Length of output: 1445
Script:
#!/bin/bash
# Let's check the implementation of useIsValidator hook
rg -A 10 "useIsValidator" packages/extension-polkagate/src/hooks/
Length of output: 2286
packages/extension-polkagate/src/fullscreen/stake/solo/StakedSolo.tsx (3)
6-7: LGTM! Clean import organization.
The import changes are well-structured, with types properly separated and the new useIsValidator hook correctly imported.
Also applies to: 15-15
40-40: LGTM! Proper hook integration.
The useIsValidator hook is correctly implemented and follows React hooks best practices.
152-152: Verify prop handling in child components.
The isValidator prop is correctly passed to both components. However, let's verify that ActiveValidators and CommonTasks components properly handle this prop to disable validator management functionality.
Also applies to: 156-156
✅ Verification successful
Let me analyze the usage of isValidator prop in both components to ensure proper handling.
Based on the comprehensive code analysis, I can now generate the final response:
The isValidator prop is properly handled in both components
Both ActiveValidators and CommonTasks components correctly implement and use the isValidator prop:
-
In
CommonTasks:- Disables validator management button when
isValidator === true - Uses prop to control warning indicators for non-validator accounts
- Properly typed as
boolean | null | undefined
- Disables validator management button when
-
In
ActiveValidators:- Shows appropriate UI states based on validator status
- Displays "This account is a validator!" message when
isValidator === true - Properly typed as
boolean | null | undefined
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of isValidator prop in child components
# Check ActiveValidators implementation
echo "Checking ActiveValidators implementation:"
ast-grep --pattern 'interface Props {
$$$
isValidator?: boolean;
$$$
}' packages/extension-polkagate/src/fullscreen/stake/solo/partials/ActiveValidators.tsx
# Check CommonTasks implementation
echo "Checking CommonTasks implementation:"
ast-grep --pattern 'interface Props {
$$$
isValidator?: boolean;
$$$
}' packages/extension-polkagate/src/fullscreen/stake/solo/partials/CommonTasks.tsx
# Check how isValidator is used in these components
echo "Checking isValidator usage:"
rg "isValidator" packages/extension-polkagate/src/fullscreen/stake/solo/partials/
Length of output: 2015
Script:
#!/bin/bash
# Get more context around the isValidator usage in CommonTasks
echo "CommonTasks isValidator usage context:"
rg "isValidator" -B 2 -A 2 packages/extension-polkagate/src/fullscreen/stake/solo/partials/CommonTasks.tsx
# Get more context around the isValidator usage in ActiveValidators
echo "ActiveValidators isValidator usage context:"
rg "isValidator" -B 2 -A 2 packages/extension-polkagate/src/fullscreen/stake/solo/partials/ActiveValidators.tsx
# Check if there are any validator-related functions or conditions
echo "Checking validator-related logic:"
rg "validator|Validator" -B 2 -A 2 packages/extension-polkagate/src/fullscreen/stake/solo/partials/
Length of output: 65709
## [0.23.1](v0.23.0...v0.23.1) (2024-11-09) ### Bug Fixes * disable manage validators if the account is already a validator ([#1634](#1634)) ([89d56c4](89d56c4))
Summary by CodeRabbit
Release Notes
New Features
StakedSolocomponent now includes validator status checks, enhancing the user experience by providing relevant information about account validation.ActiveValidatorsandCommonTaskscomponents have been updated to utilize the new validator status prop, improving rendering logic and user feedback.Bug Fixes
CommonTasksbased on the validator status, ensuring accurate representation of user actions.