-
Notifications
You must be signed in to change notification settings - Fork 13
feat: update meatadate image & deeplink changes #755
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
Conversation
fix: attempt deeplink fix for pwa
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request updates several parts of the codebase. In the payment page component, the conditional rendering for the CONFIRM view is simplified by removing an extra wrapping element and always using a centered layout. The API route file now has its import statements reordered without changing behavior. Additionally, the metadata generator now conditionally includes an application name based on the environment, and the LinkPreview component is enhanced with a new address formatting utility, type updates, and refined conditional logic for displaying an empty amount message. Changes
Sequence Diagram(s)sequenceDiagram
participant PP as PaymentPage
participant CPV as ConfirmPaymentView
PP->>PP: Determine currentView state
alt currentView is CONFIRM
PP->>CPV: Render ConfirmPaymentView directly
else
PP->>PP: Render alternative view
end
sequenceDiagram
participant LP as LinkPreviewImg
participant FA as formatDisplayAddress
LP->>FA: Validate and format Ethereum address
LP->>LP: Check if amount is empty and previewType is REQUEST
alt Amount is empty
LP->>LP: Display emptyAmountMessage
else
LP->>LP: Display standard message
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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
🧹 Nitpick comments (2)
src/app/manifest.ts (1)
1-69
: Consider implementing the new PWA manifest properties.You defined the extended manifest interface with
capture_links
andhandle_links
properties, but you're not actually using them in the returned object.return { name, short_name: name, description: 'Butter smooth global money', start_url: '/home', scope: '/', display: 'standalone', background_color: '#ffffff', theme_color: '#000000', + capture_links: 'existing-client-navigate', + handle_links: 'preferred', icons: [src/app/sw.ts (1)
74-77
: Consider enhancing client selection strategy.Currently, the code always selects the first client from the matched clients array. For a better user experience, consider implementing a more sophisticated selection strategy, such as selecting the most recently active window.
- if (clients.length > 0) { - const client = clients[0] - await client.focus() - return client.navigate(url.href) - } + if (clients.length > 0) { + // Find the most recently focused client, or default to the first one + let client = clients[0] + for (let i = 1; i < clients.length; i++) { + if (clients[i].focused) { + client = clients[i] + break + } + } + await client.focus() + return client.navigate(url.href) + }This approach is consistent with your implementation in the
notificationclick
event handler (lines 40-56).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/metadata-img.png
is excluded by!**/*.png
📒 Files selected for processing (6)
next.config.js
(1 hunks)src/app/[...recipient]/page.tsx
(1 hunks)src/app/api/apple-app-site-association/route.ts
(1 hunks)src/app/manifest.ts
(2 hunks)src/app/metadata.ts
(1 hunks)src/app/sw.ts
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/app/api/apple-app-site-association/route.ts (4)
src/app/api/assetLinks/route.ts (2) (2)
dynamic
(4-4)GET
(7-13)src/app/api/bridge/liquidation-address/get-all/route.ts (2) (2)
dynamic
(4-4)GET
(7-50)src/app/api/peanut/user/fetch-user/route.ts (2) (2)
dynamic
(6-6)GET
(8-49)src/utils/sentry.utils.ts (1) (1)
fetchWithSentry
(11-87)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (10)
next.config.js (1)
142-151
: Good security enhancement for your PWA.The addition of the
Service-Worker-Allowed: '/'
header is essential for allowing your service worker to handle all routes, which will improve your PWA's deep linking capabilities. TheX-Frame-Options: 'DENY'
header is also a good security practice to prevent clickjacking attacks.src/app/api/apple-app-site-association/route.ts (2)
3-5
: Clean up of import organization.Improved import structure with explicit imports and better organization.
10-16
: Explicit content type header improves API response correctness.Adding the explicit Content-Type header ensures browsers and clients correctly interpret the response as JSON, which is especially important for Apple App Site Association files.
src/app/[...recipient]/page.tsx (1)
154-158
: Simplified component rendering structure.The removal of conditional class assignment and unnecessary wrapper div around ConfirmPaymentView makes the code more maintainable and consistent.
src/app/manifest.ts (3)
3-7
: Enhanced PWA capabilities with deep linking support.Good addition of the
ExtendedManifest
interface to support PWA deep linking capabilities. Thecapture_links
andhandle_links
properties will improve how the app handles navigation when installed as a PWA.
9-9
: Proper type update for manifest function.Updating the return type to
ExtendedManifest
ensures type safety while providing the extended functionality.
24-24
: Correctly positioned scope property.The
scope: '/'
property is appropriately positioned and works with the new service worker permissions header you added in the next.config.js file.src/app/metadata.ts (2)
36-42
: Good addition of mobile web app capabilities.These metadata properties are essential for proper PWA functionality, particularly on iOS devices.
43-49
: Potential inconsistency in status bar style configurations.There's a discrepancy between the status bar style values:
- Line 39:
'apple-mobile-web-app-status-bar-style': 'default'
- Line 47:
statusBarStyle: 'black-translucent'
These different values for the same property could lead to inconsistent behavior across different browsers and platforms.
Consider standardizing on one style value for consistency:
other: { 'apple-mobile-web-app-capable': 'yes', 'mobile-web-app-capable': 'yes', - 'apple-mobile-web-app-status-bar-style': 'default', + 'apple-mobile-web-app-status-bar-style': 'black-translucent', 'apple-mobile-web-app-title': 'Peanut', 'application-name': 'Peanut', },src/app/sw.ts (1)
58-89
: Effective implementation of PWA deep linking.The added fetch event handler properly handles navigation requests for PWA deep linking, which is a great enhancement for the user experience.
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
🧹 Nitpick comments (2)
src/components/Global/ImageGeneration/LinkPreview.tsx (2)
19-27
: Consider adding input validation to formatDisplayAddress.The function handles different address formats well, but it doesn't check if the input is undefined, null, or empty string.
function formatDisplayAddress(address: string): string { + if (!address) return ''; if (address.startsWith('0x')) { if (isAddress(address)) { return printableAddress(address) } return address } return address }
90-111
: Improved UI conditional rendering.The conditional rendering of the amount section only when it's relevant is a good enhancement for better user experience.
Consider extracting the styling objects to constants outside the component or using a styling system like CSS modules for better maintainability in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/api/apple-app-site-association/route.ts
(1 hunks)src/app/metadata.ts
(1 hunks)src/components/Global/ImageGeneration/LinkPreview.tsx
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/api/apple-app-site-association/route.ts
- src/app/metadata.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (5)
src/components/Global/ImageGeneration/LinkPreview.tsx (5)
2-2
: Good addition of the isAddress import.This import from 'viem' provides proper address validation, which is used in the new
formatDisplayAddress
function.
9-12
: Appropriate type enhancement.Adding the optional
emptyAmountMessage
property toPreviewTypeData
is a good approach for supporting different message displays based on amount state.
14-17
: Type implementation looks good.The update to the
PREVIEW_TYPES
constant correctly implements the type changes by adding theemptyAmountMessage
field to theREQUEST
type.
46-48
: Good use of clear boolean variables.Creating explicit variables for
isEmptyAmount
andshowEmptyAmountMessage
improves code readability and makes the conditional logic clearer.
85-89
: Good conditional rendering logic.The message selection based on
showEmptyAmountMessage
is implemented correctly.
Summary by CodeRabbit