Skip to content

Add notification API for shared libraries#2778

Merged
cRui861 merged 4 commits intomainfrom
rechen/notif
Sep 7, 2023
Merged

Add notification API for shared libraries#2778
cRui861 merged 4 commits intomainfrom
rechen/notif

Conversation

@cRui861
Copy link
Member

@cRui861 cRui861 commented Aug 9, 2023

Includes a notification demo in the playground for Portal and a mock notifier (for Batch Explorer).

Screenshots from the Portal playground demo:

  • Notification popups.
    image
  • Notifications in the notification tray with the pending notification is in an updated status.
    image
  • Notifications in the notification tray with the pending notification in a completed status.
    image

Screenshots from web Playground demo

  • Using mock notification for alerts (will replace with a Batch Explorer Notifier)
    image
    image

@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Merging #2778 (96fb0c4) into main (131fe26) will decrease coverage by 0.02%.
The diff coverage is 59.34%.

❗ Current head 96fb0c4 differs from pull request most recent head a210bf9. Consider uploading reports for the commit a210bf9 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2778      +/-   ##
==========================================
- Coverage   66.47%   66.45%   -0.02%     
==========================================
  Files        1229     1232       +3     
  Lines       33744    33835      +91     
  Branches     6158     6160       +2     
==========================================
+ Hits        22431    22485      +54     
- Misses      11179    11216      +37     
  Partials      134      134              
Files Changed Coverage Δ
packages/playground/src/layout/demo-nav-menu.tsx 100.00% <ø> (ø)
...ges/bonito-core/src/notification/alert-notifier.ts 8.33% <8.33%> (ø)
...ges/playground/src/demo/form/notification-demo.tsx 27.27% <27.27%> (ø)
...es/bonito-core/src/environment/mock-environment.ts 83.78% <50.00%> (-1.94%) ⬇️
packages/playground/src/demo-routes.tsx 56.75% <50.00%> (-0.39%) ⬇️
...ages/bonito-core/src/notification/fake-notifier.ts 84.61% <84.61%> (ø)
...ackages/bonito-core/src/environment/environment.ts 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 131fe26...a210bf9. Read the comment docs.

📢 Have feedback on the report? Share it here.

@gingi
Copy link
Member

gingi commented Aug 16, 2023

Rena, can you provide a screenshot or two of the demo?

Copy link
Member

@gingi gingi left a comment

Choose a reason for hiding this comment

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

The PR looks great, but I think renaming "Notification" to "Notifier" will avoid some confusion in the future.

@cRui861 cRui861 merged commit 1fa572a into main Sep 7, 2023
@cRui861 cRui861 deleted the rechen/notif branch September 7, 2023 22:38
gingi pushed a commit that referenced this pull request Nov 9, 2023
* WIP Notification fakes

* Move notification files over to bonito-core

* Fix paths

* Update naming from notification to notifier
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