-
Notifications
You must be signed in to change notification settings - Fork 5
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
style: add wordpress loading #1605
Conversation
WalkthroughThe pull request introduces changes to the Changes
Possibly related PRs
Suggested reviewers
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: 2
🧹 Outside diff range and nitpick comments (4)
packages/extension-polkagate/src/components/Progress.tsx (2)
40-45
: Consider adding aria-label for accessibility.The implementation looks good, but consider adding accessibility attributes for screen readers.
{type === 'wordpress' && <Wordpress color={theme.palette.primary.main} size={size} + aria-label="Loading" /> }
Line range hint
21-78
: Consider splitting spinner implementations for better maintainability.The component currently handles multiple spinner types with different configurations. Consider extracting each spinner type into its own component for better maintainability and type safety.
Example structure:
// CircleSpinner.tsx, WordpressSpinner.tsx, etc. const CircleSpinner = ({ size, color }: CircleSpinnerProps) => ( <Circle size={size} color={color} ... /> ); // Progress.tsx const spinnerComponents = { circle: CircleSpinner, wordpress: WordpressSpinner, // ... }; function Progress({ type = 'circle', ...props }: Props) { const SpinnerComponent = spinnerComponents[type]; return ( <Grid ...> <SpinnerComponent {...props} /> {/* title rendering */} </Grid> ); }packages/extension-polkagate/src/popup/history/index.tsx (1)
81-89
: LGTM! Consider adding aria-label for accessibility.The replacement of static text with the Progress component improves the loading state visualization. The configuration with horizontal layout and padding adjustments provides a clean inline loading experience.
Consider adding an aria-label to improve accessibility:
<Progress direction='row' pt='5px' size={15} title={t('Loading...')} titlePaddingLeft={5} titlePaddingTop={0} type='wordpress' + aria-label={t('Loading more transactions')} />
packages/extension-polkagate/src/popup/history/modal/HistoryModal.tsx (1)
88-96
: The Progress component implementation looks good, with some enhancement opportunities.The loading indicator implementation is functionally correct and aligns with the PR objective of adding WordPress-style loading. However, consider these improvements for better UX:
Consider these enhancements:
<Progress direction='row' pt='5px' - size={15} + size={20} title={t('Loading...')} titlePaddingLeft={5} titlePaddingTop={0} type='wordpress' />
- Increase the size slightly for better visibility
- Consider using a more descriptive loading message like "Loading more transactions..."
- Consider extracting the padding values to theme constants if they're used elsewhere
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/extension-polkagate/src/components/Progress.tsx (3 hunks)
- packages/extension-polkagate/src/popup/history/index.tsx (1 hunks)
- packages/extension-polkagate/src/popup/history/modal/HistoryModal.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
packages/extension-polkagate/src/components/Progress.tsx (2)
10-18
: LGTM! Well-structured interface updates.The new props are properly typed and provide good flexibility for layout customization.
21-21
: LGTM! Good default values maintaining backward compatibility.The default values are well-chosen and ensure existing usage won't break.
Summary by CodeRabbit
New Features
Progress
component with new layout options and a 'wordpress' spinner type.TransactionHistory
andHistoryModal
components, replacing text with a visualProgress
component for better user feedback.Bug Fixes