-
Notifications
You must be signed in to change notification settings - Fork 138
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
update provider.tsx #944
update provider.tsx #944
Conversation
@godekina is attempting to deploy a commit to the LFG Labs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Wallet
User->>App: Check for wallet installation
App->>Wallet: Call isBitgetWalletInstalled()
Wallet-->>App: Return boolean
App->>User: Display wallet status
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
app/provider.tsxOops! Something went wrong! :( ESLint: 9.14.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by 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: 0
🧹 Outside diff range and nitpick comments (4)
app/provider.tsx (4)
Line range hint
25-35
: Consider improving type safety and privacy settings.
- The type casting of
window
can be improved using a more specific type.- Session recording with
recordCrossOriginIframes
enabled may have privacy implications.Consider these improvements:
- (window as any).posthog = posthog; + interface CustomWindow extends Window { + posthog: typeof posthog; + } + (window as CustomWindow).posthog = posthog;Also, review if cross-origin iframe recording is necessary for your use case, as it may capture sensitive user data.
37-40
: Improve type safety and maintain consistency in quotes.The function uses double quotes while the rest of the file uses single quotes.
Consider this improvement:
function isBitgetWalletInstalled() { // Check if the Bitget wallet is available on the window object - return typeof window !== "undefined" && window.bitget !== undefined; + return typeof window !== 'undefined' && window.bitget !== undefined; }
45-53
: Consider extracting the base64 icon to improve readability.The large base64 icon string makes the code harder to read and maintain.
Consider moving it to a separate constants file:
+ // constants/icons.ts + export const BITGET_WALLET_ICON = 'data:image/png;base64,...'; // provider.tsx + import { BITGET_WALLET_ICON } from '@constants/icons'; new InjectedConnector({ options: { id: "bitkeep", name: isBitgetWalletInstalled() ? "Bitget Wallet" : "Install Bitget Wallet", - icon: "data:image/png;base64,..." + icon: BITGET_WALLET_ICON }, }),
86-130
: Consider improving theme configuration maintainability.
- Hardcoded color values should be moved to theme tokens
- Media query syntax could be improved using MUI's breakpoint utilities
Consider these improvements:
+ // Define color tokens + const colors = { + primary: { + main: '#6affaf', + light: '#5ce3fe', + }, + secondary: { + main: '#f4faff', + dark: '#eae0d5', + }, + text: { + unselected: '#E1DCEA', + } + }; const theme = createTheme({ palette: { - primary: { - main: '#6affaf', - light: '#5ce3fe', - }, + primary: colors.primary, - secondary: { - main: '#f4faff', - dark: '#eae0d5', - }, + secondary: colors.secondary, }, components: { MuiTabs: { styleOverrides: { root: { '& .MuiTabs-flexContainer': { display: 'flex', flexDirection: 'column', alignItems: 'center', - ['@media (min-width:768px)']: { + [theme.breakpoints.up('md')]: { flexDirection: 'row', }, }, }, }, }, MuiTab: { styleOverrides: { root: { - color: '#E1DCEA', + color: colors.text.unselected, width: '100%', - ['@media (min-width:768px)']: { + [theme.breakpoints.up('md')]: { width: 'fit-content', }, }, }, }, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
app/provider.tsx
(4 hunks)
🔇 Additional comments (1)
app/provider.tsx (1)
Line range hint 137-156
: LGTM! Well-structured provider composition.
The provider composition is clean, well-organized, and follows React best practices.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 for a quick fix, we'll refactor everything later to make it something more organized
id: "bitkeep", | ||
name: isBitgetWalletInstalled() | ||
? "Bitget Wallet" | ||
: "Install Bitget Wallet", |
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.
That will work for now but I think we need to completely reorganize the way we're managing wallets following app.starknet.id model
Bug fix
The bug that make the logo not to appear now works, and the messaging has been changed to Install bitget wallet.
@fricoben kindly review
Summary by CodeRabbit
New Features
Bug Fixes