fix(native-app): more license fixes #17699
Conversation
WalkthroughThis pull request introduces a new feature flag Changes
Sequence DiagramsequenceDiagram
participant User
participant WalletPassScreen
participant FeatureFlag
User->>WalletPassScreen: Loads screen
WalletPassScreen->>FeatureFlag: Check isAddToWalletButtonEnabled
FeatureFlag-->>WalletPassScreen: Return flag status
alt Flag is true
WalletPassScreen->>User: Display "Add to Wallet" button
else Flag is false
WalletPassScreen->>User: Hide "Add to Wallet" button
end
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
Documentation and Community
|
![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
🧹 Nitpick comments (1)
apps/native/app/src/screens/wallet-pass/wallet-pass.tsx (1)
190-204: Consider extracting spacing calculations into separate functions.The spacing calculations are complex and would be more maintainable if extracted into named functions.
+const calculateInformationTopSpacing = ( + allowLicenseBarcode: boolean, + loading: boolean, + hasBarcode: boolean, + isConnected: boolean, + error: boolean, + isBarcodeEnabled: boolean, + barcodeHeight: number, + theme: Theme +) => { + return (allowLicenseBarcode && ((loading && !hasBarcode) || hasBarcode)) || + ((!isConnected || error) && isBarcodeEnabled) + ? barcodeHeight + LICENSE_CARD_ROW_GAP + theme.spacing[4] + : theme.spacing[2] +} + +const calculateBottomInset = ( + informationTopSpacing: number, + isConnected: boolean, + isBarcodeEnabled: boolean, + isTablet: boolean, + allowLicenseBarcode: boolean, + isAddToWalletButtonEnabled: boolean +) => { + return informationTopSpacing || (!isConnected && isBarcodeEnabled) + ? isTablet + ? 340 + : !allowLicenseBarcode + ? 80 + : isAddToWalletButtonEnabled + ? 182 + : 192 + : 0 +} - const informationTopSpacing = - (allowLicenseBarcode && ((loading && !data?.barcode) || data?.barcode)) || - ((!isConnected || res.error) && isBarcodeEnabled) - ? barcodeHeight + LICENSE_CARD_ROW_GAP + theme.spacing[4] - : theme.spacing[2] - - const bottomInset = - informationTopSpacing || (!isConnected && isBarcodeEnabled) - ? isTablet - ? 340 - : !allowLicenseBarcode - ? 80 - : isAddToWalletButtonEnabled - ? 182 - : 192 - : 0 + const informationTopSpacing = calculateInformationTopSpacing( + allowLicenseBarcode, + loading, + !!data?.barcode, + isConnected, + !!res.error, + isBarcodeEnabled, + barcodeHeight, + theme + ) + + const bottomInset = calculateBottomInset( + informationTopSpacing, + isConnected, + isBarcodeEnabled, + isTablet, + allowLicenseBarcode, + isAddToWalletButtonEnabled + )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/native/app/src/screens/wallet-pass/wallet-pass.tsx(5 hunks)apps/native/app/src/ui/lib/card/license-card.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/native/app/src/ui/lib/card/license-card.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/screens/wallet-pass/wallet-pass.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (6)
apps/native/app/src/ui/lib/card/license-card.tsx (2)
40-43: LGTM! Well-implemented dynamic margin.The styled component now properly handles dynamic margin based on props, improving layout flexibility.
185-185: LGTM! Proper usage of the dynamic margin.The margin is correctly calculated using theme spacing and conditional logic.
apps/native/app/src/screens/wallet-pass/wallet-pass.tsx (4)
152-155: LGTM! Well-implemented feature flag.The feature flag follows the existing pattern and maintains backward compatibility with a default value of true.
210-210: LGTM! Proper useEffect dependency update.The isAddToWalletButtonEnabled flag is correctly added to the dependencies array since it affects the layout.
444-444: LGTM! Proper usage of bottomInset.The contentInset is correctly updated to use the calculated bottomInset value.
495-495: LGTM! Proper implementation of the feature flag in button rendering.The button visibility is correctly controlled by the isAddToWalletButtonEnabled flag while maintaining the existing animation logic.
Also applies to: 505-505
* fix: spacing for license info and add to wallet button * feat: add feature flag to show/hide add to wallet button * fix: spacing on license card
* fix: spacing for license info and add to wallet button * feat: add feature flag to show/hide add to wallet button * fix: spacing on license card
What
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Improvements
These changes provide more control over UI elements and enhance the overall user experience with more dynamic component rendering.