chore: add support us modal #1620
Conversation
WalkthroughThe changes introduce a new Changes
Possibly related PRs
Suggested labels
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: 6
🧹 Outside diff range and nitpick comments (1)
packages/extension-polkagate/src/components/TwoButtons.tsx (1)
65-65: Consider preserving icon placement in busy state.
While the implementation is correct, consider maintaining visual consistency by showing a placeholder for the icon in the busy state to prevent layout shifts.
Here's how you could enhance the busy state:
? <Grid alignItems='center' aria-label='primaryBusyButton' container justifyContent='center' role='button' sx={{ backgroundColor: 'secondary.main', border: '1px solid', borderColor: 'secondary.main', borderRadius: '5px', fontSize: '18px', fontWeight: 400, height: '36px', textTransform: 'none', width: '100%' }}>
+ {primaryBtnStartIcon && <div style={{ visibility: 'hidden', width: 20 }}>{primaryBtnStartIcon}</div>}
<Circle color='white' scaleEnd={0.7} scaleStart={0.4} size={25} />
</Grid>📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- packages/extension-polkagate/src/components/TwoButtons.tsx (4 hunks)
- packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx (1 hunks)
- packages/extension-polkagate/src/fullscreen/homeFullScreen/index.tsx (2 hunks)
- packages/extension/public/locales/en/translation.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/extension/public/locales/en/translation.json
🔇 Additional comments (6)
packages/extension-polkagate/src/components/TwoButtons.tsx (4)
19-20: LGTM! Props interface changes are well-structured.
The new optional icon props are properly typed and maintain backward compatibility.
30-30: LGTM! Function signature is properly updated.
The new props are cleanly integrated while maintaining alphabetical ordering and existing defaults.
40-40: LGTM! Secondary button icon implementation is correct.
The startIcon prop is properly integrated with the MUI Button component.
19-20: Verify impact on existing TwoButtons usage.
Since this component might be used in multiple places, let's verify that the icon additions don't affect existing implementations.
Also applies to: 30-30, 40-40, 65-65
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough information about all usages of the TwoButtons component across the codebase.
Icon props are safe additions that don't affect existing usage
The addition of optional icon props (primaryBtnStartIcon and secondaryBtnStartIcon) is backward compatible since:
- All existing usages of TwoButtons (found in 45+ locations) don't use these icon props
- The props are marked as optional with
?modifier - Only the new SupportUs modal utilizes the icon prop:
primaryBtnStartIcon={<ThumbUpIcon />}
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing TwoButtons usage to verify compatibility
rg -l "TwoButtons" --type tsx | xargs rg "<TwoButtons" -B 2 -A 5
Length of output: 57800
packages/extension-polkagate/src/fullscreen/homeFullScreen/index.tsx (2)
13-13: LGTM!
The import statement follows the existing pattern and maintains proper component organization.
93-93: Verify the modal's visibility management.
The SupportUs component is unconditionally rendered. Please ensure it properly manages its own visibility to avoid showing the modal at inappropriate times.
Let's verify the component's visibility logic:
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
Outdated
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
Outdated
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
Outdated
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
Outdated
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
Outdated
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (3)
packages/extension-polkagate/src/hooks/useMyVote.ts (1)
Line range hint 19-44: Enhance error handling and optimize effects.
The implementation has several areas for improvement:
- Error handling only logs to console without proper error recovery
- Two separate effects might be consolidated
- The refresh mechanism could be simplified
Consider these improvements:
const { api, formatted } = useInfo(address);
const [vote, setVote] = useState<Vote | null | undefined>();
+const [error, setError] = useState<Error | null>(null);
const fetchVote = useCallback(async () => {
+ setError(null);
try {
if (formatted && api && refIndex !== undefined && trackId !== undefined) {
const vote = await getAddressVote(String(formatted), api, Number(refIndex), Number(trackId));
setVote(vote);
- setRefresh?.(false);
+ onVoteUpdate?.();
}
} catch (error) {
- console.error(error);
+ setError(error instanceof Error ? error : new Error('Failed to fetch vote'));
+ setVote(null);
}
}, [api, formatted, refIndex, trackId]);
useEffect(() => {
- fetchVote().catch(console.error);
-}, [fetchVote]);
-
-useEffect(() => {
- refresh && fetchVote().catch(console.error);
-}, [fetchVote, refresh]);
+ if (refresh || !vote) {
+ fetchVote();
+ }
+}, [fetchVote, refresh, vote]);
-return vote;
+return error ? null : vote;This would:
- Add proper error state management
- Consolidate the effects into a single, more efficient one
- Provide better error recovery mechanisms
packages/extension-polkagate/src/fullscreen/governance/post/myVote/util.tsx (2)
Line range hint 99-107: Add error handling for proxy voting queries.
The proxy voting logic should handle potential failures when querying the delegated target's votes.
- const proxyVoting = await api.query['convictionVoting']['votingFor'](target, trackId) as unknown as PalletConvictionVotingVoteVoting;
- const targetVote = proxyVoting.isCasting ? proxyVoting.asCasting.votes.find(([index]) => index.toNumber() === referendumIndex)?.[1] : undefined;
+ try {
+ const proxyVoting = await api.query.convictionVoting.votingFor(target, trackId) as PalletConvictionVotingVoteVoting;
+ const targetVote = proxyVoting.isCasting ? proxyVoting.asCasting.votes.find(([index]) => index.toNumber() === referendumIndex)?.[1] : undefined;
+ } catch (error) {
+ console.error('Failed to fetch proxy voting data:', error);
+ return null;
+ }Line range hint 142-159: Clean up commented code and add error handling.
The function has several issues to address:
- Remove or implement the commented-out delegation logic
- Add error handling for Promise.all
- Add type guards for the returned values
export async function getAllVotes (address: string, api: ApiPromise, tracks: Track[]): Promise<number[] | null> {
const queries = tracks.map((t) => api.query['convictionVoting']['votingFor'](address, t[0]));
- const voting = await Promise.all(queries);
+ try {
+ const voting = await Promise.all(queries);
+ const castedRefIndexes = voting?.map((v) => {
+ const jsonV = v.toJSON() as unknown as Voting;
+ return jsonV?.casting?.votes?.map((vote) => vote[0]) ?? [];
+ });
+ return castedRefIndexes.flat().filter((index): index is number => typeof index === 'number');
+ } catch (error) {
+ console.error('Failed to fetch votes:', error);
+ return null;
+ }
- const castedRefIndexes = voting?.map((v) => {
- const jsonV = v.toJSON() as unknown as Voting;
- return jsonV?.casting?.votes?.map((vote) => vote[0]);
- });
- // if (jsonVoting.delegating) {
- // // Then, look into the votes of the delegating target address.
- // const { target, conviction } = jsonVoting.delegating;
- // const proxyVoting = await api.query.convictionVoting.votingFor(target, trackId);
- // const jsonProxyVoting = proxyVoting?.toJSON() as Voting;
- // const vote = jsonProxyVoting?.casting?.votes?.find(([index]) => index === referendumIndex)?.[1];
- // }
- return castedRefIndexes.flat();
}📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx (1 hunks)
- packages/extension-polkagate/src/fullscreen/governance/post/myVote/util.tsx (4 hunks)
- packages/extension-polkagate/src/hooks/useMyVote.ts (2 hunks)
- packages/extension/public/locales/en/translation.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension/public/locales/en/translation.json
🔇 Additional comments (4)
packages/extension-polkagate/src/hooks/useMyVote.ts (2)
3-10: LGTM! Clean import organization.
The imports are well-organized, separating external React dependencies from internal utilities.
Line range hint 12-44: Verify hook usage across the codebase.
The changes to error handling and the refresh mechanism might affect existing implementations.
Let's verify the hook usage:
✅ Verification successful
No breaking changes detected in hook usage
The changes to error handling in useMyVote hook are safe and backward compatible:
- Both call sites use the hook correctly with the expected parameters
- Both implementations handle the hook's return value appropriately through
useMemohooks - The error handling changes in the hook are internal and don't affect the consumers
- The refresh mechanism is only used in
post/index.tsxwhich properly passes both required parameters
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for useMyVote hook usage
rg -A 5 "useMyVote\(" --type ts --type tsx
Length of output: 72
Script:
#!/bin/bash
# Search for useMyVote hook usage with correct file extensions
rg "useMyVote\(" -g '*.{ts,tsx}'
# Also search for potential imports of useMyVote
rg "import.*useMyVote" -g '*.{ts,tsx}'
Length of output: 701
Script:
#!/bin/bash
# Let's examine the usage context in these files
echo "=== SupportUs.tsx usage ==="
rg -B 5 -A 10 "useMyVote\(" packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
echo -e "\n=== post/index.tsx usage ==="
rg -B 5 -A 10 "useMyVote\(" packages/extension-polkagate/src/fullscreen/governance/post/index.tsx
Length of output: 2067
packages/extension-polkagate/src/fullscreen/governance/post/myVote/util.tsx (1)
6-10: Well-organized type imports!
Good use of type-only imports which helps with tree-shaking and makes the code's intent clearer.
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx (1)
23-25: Previous suggestion about configurability is still applicable
The constants PROPOSAL_NO and SHOW_INTERVAL are hard-coded, which reduces flexibility. Consider making them configurable through environment variables or component props to allow easier updates without code changes.
packages/extension-polkagate/src/fullscreen/governance/post/myVote/util.tsx
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/post/myVote/util.tsx
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
Outdated
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
Outdated
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
Outdated
Show resolved
Hide resolved
* feat: popup * fix: resolve confilicts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx (2)
171-174: Enhance image accessibility.
The alt text for the support image could be more descriptive to improve accessibility.
Update the alt text to be more descriptive:
-alt='Support PolkaGate'
+alt={t('PolkaGate governance support illustration showing community collaboration')}143-157: Add ARIA labels to improve dialog accessibility.
The dialog component should have proper ARIA labels for better screen reader support.
Add appropriate ARIA labels:
<Dialog
+ aria-labelledby="support-dialog-title"
+ aria-describedby="support-dialog-description"
PaperProps={{
style: {
background: 'transparent',
borderRadius: 24,
overflow: 'hidden'
}
}}
TransitionComponent={Slide}
fullWidth
maxWidth='sm'
onClose={handleClose}
open={open}
>Then update the corresponding text elements:
-<Typography fontSize='35px' fontWeight={500} sx={{ color: theme.palette.primary.contrastText }}>
+<Typography id="support-dialog-title" fontSize='35px' fontWeight={500} sx={{ color: theme.palette.primary.contrastText }}>
{t('Support PolkaGate')}
</Typography>
-<Typography fontSize='16px' fontWeight={400} sx={{ color: '#fff', pb: '20px' }}>
+<Typography id="support-dialog-description" fontSize='16px' fontWeight={400} sx={{ color: '#fff', pb: '20px' }}>
{t("We're seeking retroactive funding to sustain and expand PolkaGate's impact!")}
</Typography>📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx (1 hunks)
🧰 Additional context used
🪛 GitHub Check: build
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
[failure] 90-90:
'timeToShow' is declared but its value is never read.
[failure] 92-92:
'notVoted' is declared but its value is never read.
🔇 Additional comments (1)
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx (1)
27-81: LGTM! Well-structured styled components.
The styled components follow MUI best practices with proper theme integration and responsive design considerations.
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
Outdated
Show resolved
Hide resolved
plus adding background blurriness
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/extension/public/locales/zh/translation.json (1)
1408-1408: Consider standardizing translation style for consistency.The translation "无代理" could be made more consistent with other similar translations by adding a common suffix (e.g., "无代理账户"). Additionally, consider standardizing the use of quotation marks throughout the file for better consistency.
- "No proxy": "无代理", + "No proxy": "无代理账户",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx(1 hunks)packages/extension/public/locales/fr/translation.json(1 hunks)packages/extension/public/locales/hi/translation.json(1 hunks)packages/extension/public/locales/ru/translation.json(1 hunks)packages/extension/public/locales/zh/translation.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/extension/public/locales/fr/translation.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
🔇 Additional comments (4)
packages/extension/public/locales/zh/translation.json (1)
1409-1420: LGTM! New support modal translations are well-implemented.
The Chinese translations for the support modal maintain accuracy while being culturally appropriate. The messaging about retroactive funding and voting is clear and persuasive.
packages/extension/public/locales/hi/translation.json (2)
1401-1403: LGTM! Basic UI translations look good.
The translations for "No proxy", "Switch Theme" and "Account Icon" are accurate and consistent with Hindi language conventions.
1404-1413: LGTM! Support modal translations are well-structured and accurate.
The translations maintain proper Hindi language conventions while accurately conveying the original meaning. The text is properly formatted with appropriate punctuation and spacing.
packages/extension/public/locales/ru/translation.json (1)
1399-1411: LGTM! The Russian translations are accurate and well-formatted.
The new translations maintain the original meaning while being culturally appropriate for Russian speakers. The formatting and punctuation are consistent with the rest of the file.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (1)
packages/extension-polkagate/src/fullscreen/governance/old-SupportUs.tsx (1)
150-229: Add accessibility improvements to the dialog.The dialog component could benefit from accessibility improvements.
Consider adding these accessibility enhancements:
<Dialog + aria-describedby="support-us-description" + aria-labelledby="support-us-title" PaperProps={{ style: { background: 'transparent', borderRadius: 24, overflow: 'hidden' } }} // ... other props > <DialogContent style={{ padding: 0 }}> <StyledPaper theme={theme}> <ContentWrapper> <Box alignItems='center' display='flex' mb={3}> <HandshakeIcon sx={{ color: theme.palette.approval.contrastText, fontSize: 40, mr: 2 }} /> - <Typography fontSize='35px' fontWeight={500} sx={{ color: theme.palette.approval.contrastText }}> + <Typography + id="support-us-title" + fontSize='35px' + fontWeight={500} + sx={{ color: theme.palette.approval.contrastText }} + > {t('Support PolkaGate')} </Typography> </Box> - <Typography fontSize='16px' fontWeight={400} sx={{ color: '#fff', pb: '20px' }}> + <Typography + id="support-us-description" + fontSize='16px' + fontWeight={400} + sx={{ color: '#fff', pb: '20px' }} + > {t("We're seeking retroactive funding to sustain and expand PolkaGate's impact!")} </Typography>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx(1 hunks)packages/extension-polkagate/src/fullscreen/governance/old-SupportUs.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension-polkagate/src/fullscreen/governance/SupportUs.tsx
🔇 Additional comments (1)
packages/extension-polkagate/src/fullscreen/governance/old-SupportUs.tsx (1)
26-82: LGTM! Well-structured styled components.
The styled components are well-implemented with proper theming, transitions, and hover effects.
packages/extension-polkagate/src/fullscreen/governance/old-SupportUs.tsx
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/old-SupportUs.tsx
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/old-SupportUs.tsx
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/old-SupportUs.tsx
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/old-SupportUs.tsx
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
Localization
Improvements