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.

Sign up for GitHub

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

Jump to bottom

feat: mark notification as read if visible #2532

Merged
merged 5 commits into from
May 23, 2024
Merged

feat: mark notification as read if visible #2532

merged 5 commits into from
May 23, 2024

Conversation

jeeanribeiro
Copy link
Member

@jeeanribeiro jeeanribeiro commented May 23, 2024 •
edited
Loading

Summary

⚠️ depends on https://github.com/bloomwalletio/bloom-ui/pull/326

Uses Intersection Observer API for observing the notification's elements visibility and to mark them as read.
It was necessary to add visibilityChange on Popover to recalculate the observed targets' intersection status every time the Popover visibility is changed.

closes #2524

Testing

Platforms

  • Desktop
    • MacOS
    • Linux
    • Windows

Checklist

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or modified tests that prove my changes work as intended
  • I have verified that new and existing unit tests pass locally with my changes
  • I have verified that my latest changes pass CI workflows for testing and linting
  • I have made corresponding changes to the documentation

@jeeanribeiro jeeanribeiro requested a review from MarkNerdi May 23, 2024 01:32
@jeeanribeiro jeeanribeiro self-assigned this May 23, 2024
@jeeanribeiro jeeanribeiro linked an issue May 23, 2024 that may be closed by this pull request
️🛠 - Mark notification as Read if visible #2524
Closed
@MarkNerdi MarkNerdi merged commit 27440f3 into feature/notifications May 23, 2024
3 of 5 checks passed
@MarkNerdi MarkNerdi deleted the 2524-mark-notification-as-read-if-visible branch May 23, 2024 13:32
nicole-obrien added a commit that referenced this pull request Jun 12, 2024
@MarkNerdi @nicole-obrien
@Tuditi @jeeanribeiro
feat: notifications (#2550)
a1a8a86 
* feat: add walletconnect notify api (#2512)

* initialize notify client

* notify client store

* register and subscribe to notifications

* implement test subscription

* improve initialization

* subscribe to gm walletconnect

* create notify manager class

Co-authored-by: Nicole O'Brien <[email protected]>
Co-authored-by: Tuditi <[email protected]>

* set notifications to store

* update yarn lock

* initialize notifications and subscriptions

Co-authored-by: Tuditi <[email protected]>
Co-authored-by: Nicole O'Brien <[email protected]>

* cleanup handlers

Co-authored-by: Tuditi <[email protected]>
Co-authored-by: Nicole O'Brien <[email protected]>
Co-authored-by: Jean Ribeiro <[email protected]>

---------

Co-authored-by: Nicole O'Brien <[email protected]>
Co-authored-by: Tuditi <[email protected]>
Co-authored-by: Jean Ribeiro <[email protected]>

* chore: cleanup notification manager init (#2513)

* cleanup notification manager initialization

Co-authored-by: Nicole O'Brien <[email protected]>

* add message handler

---------

Co-authored-by: Nicole O'Brien <[email protected]>

* fix: fix notify client (#2516)

* fix notify client initialize

* fix dependencies

Co-authored-by: Jean Ribeiro <[email protected]>

---------

Co-authored-by: Jean Ribeiro <[email protected]>

* feat: add notification components (#2514)

* WIP

* improve notification popup

* use dapp icon as main image

* add small badge

* add notificationavatar

* only show fixed amount + add localizations

Co-authored-by: Jean Ribeiro <[email protected]>

* add time test

* cleanup

---------

Co-authored-by: Jean Ribeiro <[email protected]>

* feat: adds Link in NotificationTile (#2528)

* feat: highlight unread notifications (#2527)

* chore: refactor for multi chain support (#2511)

* chore: remove BASE_TOKEN_CONTRACT_ADDRESS map

* chore: rename getL1Network to getStardustNetwork

* fix: type

* feat: highlight unread notification

* fix incoming notifications

* restructure

---------

Co-authored-by: Tuditi <[email protected]>
Co-authored-by: Mark Nardi <[email protected]>

* feat: mark notification as read if visible (#2532)

* fix: removes view all button and show all notifications in dropdown

* feat: mark notifications as read if visible

* chore: update @bloomwalletio/ui

* fix: updates notifications store only after closing the dropdown

* decrease threshold

---------

Co-authored-by: Mark Nardi <[email protected]>

* fix: type errors

* feat: notifications unread tab (#2540)

* feat: notifications unread tab

* add optional chain

---------

Co-authored-by: Mark Nardi <[email protected]>

* enhancement: uses VirtualList in notifications (#2541)

* enhancement: uses VirtualList in notifications

* fix merge issue

---------

Co-authored-by: Mark Nardi <[email protected]>

* cleanup

* feat: notification details popup (#2554)

* show basetoken amount on smartcontract popups (#2549)

* feat: adds notification details popup

Co-authored-by: Mark Nardi <[email protected]>
Co-authored-by: Nicole O'Brien <[email protected]>
Co-authored-by: Tuditi <[email protected]>

* fix: PopupComponentMap type

---------

Co-authored-by: Mark Nardi <[email protected]>
Co-authored-by: Nicole O'Brien <[email protected]>
Co-authored-by: Tuditi <[email protected]>

* fix linter

* fix: notification read state

Co-authored-by: Mark Nardi <[email protected]>

* polish nav bar with notification button

* split notification components

* remove notification tabs

* enable notifications for multiple networks

* fix notifications initialization

* improve siwe validation check

* fix: notification tile hover

* fix: popover closing behaviour for notifications

* ui: empty placeholder in notifications

* enable notifications in network drawer

* fix notification toggle state

* improve notification registering

Co-authored-by: Mark Nardi <[email protected]>

* register for notifications on some generate addresses

* register account on evm address generation

* only register if not registered

* fix login if account is unregistered

* Fix notification registration on onboarding

Co-authored-by: Nicole O'Brien <[email protected]>

* remove mulitple instances of notifiation body

* WIP

* fix: whitelist notification links

* fix tracked address initialization

* remove console logs

* ui: polish notification details popup

Co-authored-by: Mark Nardi <[email protected]>

* ui: polish notification tile

* improve logic in complete onboarding

Co-authored-by: Mark Nardi <[email protected]>

* chore: fix formatting

---------

Co-authored-by: Nicole O'Brien <[email protected]>
Co-authored-by: Tuditi <[email protected]>
Co-authored-by: Jean Ribeiro <[email protected]>
Co-authored-by: Tuditi <[email protected]>
Co-authored-by: Nicole O'Brien <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarkNerdi MarkNerdi MarkNerdi approved these changes

@cvarley100 cvarley100 Awaiting requested review from cvarley100

@nicole-obrien nicole-obrien Awaiting requested review from nicole-obrien

Assignees

@jeeanribeiro jeeanribeiro

Labels
type:feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

️🛠 - Mark notification as Read if visible
2 participants
@jeeanribeiro @MarkNerdi