Skip to content
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

feat(wallet-mobile): Add in-app notifications #3775

Merged
merged 11 commits into from
Jan 6, 2025

Conversation

michaeljscript
Copy link
Collaborator

@michaeljscript michaeljscript commented Jan 1, 2025

Description / Change(s) / Related issue(s)

Add in app notifications

Ticket

YV-157

@michaeljscript
Copy link
Collaborator Author

Let's merge #3744 first. Note the target branch is not develop

Base automatically changed from feat/remote-notifications to develop January 2, 2025 13:25
@michaeljscript michaeljscript marked this pull request as ready for review January 6, 2025 14:14
pushEvent(event)
}

if (event.trigger === Notifications.Trigger.TransactionReceived && event.metadata.walletId === selectedWalletId) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

topic to discuss with product, since if you have wallet X open and Y is syncing in background, should we display the other wallet messages? otherwise when switching the wallet it will send all notifications in one shot E.g if the wallet Y received 10 txs while X was open. (not a blocker for this PR tho)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have a look

const {styles} = useStyles()
return (
<View style={styles.absolute}>
<SafeAreaView edges={['top']}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it a Screen? if it is please add to the file name (convention)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a screen, just the layout where the in-app notification is shown. Similar to our modals.

@stackchain stackchain added this to the 5.3.0 milestone Jan 6, 2025
@stackchain stackchain merged commit 0ce90f4 into develop Jan 6, 2025
1 check passed
@stackchain stackchain deleted the feat/in-app-notifications branch January 6, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants