Opened 4 years ago

Last modified 2 days ago

#2386 new change

Make it possible to register a notification download listener

Reported by: fhd Assignee:
Priority: P2 Milestone:
Module: Core Keywords:
Cc: sebastian Blocked By:
Blocking: Platform: Unknown
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by trev)

Background

In #2366, we want to react to notification downloads - there's currently no non-hacky way to do that.

What to change

Add API for registering a notification download listener:

  • Notification.addNewNotificationListener()
  • Notification.removeNewNotificationListener()

The listeners should be triggered after downloading notifications but only if that download changed the result that Notification.getNextToShow() would return.

Change History (21)

comment:1 Changed 4 years ago by fhd

  • Cc trev added

Wladimir: Not set to ready yet, please have a look first.

comment:2 Changed 4 years ago by fhd

  • Blocking 2387 added

comment:3 Changed 4 years ago by trev

  • Description modified (diff)
  • Ready set

comment:4 Changed 4 years ago by sebastian

  • Cc sebastian added

For consistency (and personal preference) I'd rather go with:

  • Notification.addListener
  • Notification.removeListener

Or if this clashes with other notification related events:

  • Notification.onNewNofication.addListener
  • Notification.onNewNofication.removeListener

comment:5 Changed 4 years ago by fhd

While I don't completely like what's in the description either (e.g. not consistent with addQuestionListener), I can't really come up with anything better (addNewListener would be quite confusing e.g.). onNewNotification.addListener would be even more inconsisten than addNewNotificationListener.

Version 1, edited 4 years ago by fhd (previous) (next) (diff)

comment:6 Changed 4 years ago by sebastian

Alright, wasn't aware of addQuestionListener but thinking of the rest of our code.

comment:7 Changed 4 years ago by trev

Note that we aren't using event targets in the core code at this point. While I agree that we want to change the API in that direction (and resolve the inconsistency between Chrome's Prefs implementation and all other platforms), that change won't happen here.

comment:8 Changed 4 years ago by fhd

  • Blocking 2419 added

comment:9 Changed 4 years ago by fhd

  • Blocking 2367 removed

comment:10 Changed 4 years ago by fhd

  • Blocking 2368 removed

comment:11 Changed 4 years ago by fhd

  • Blocking 2387 removed

comment:12 Changed 4 years ago by fhd

  • Ready unset

Removed Ready for now. Since we agreed that we want to move the notification display logic to Core (#2366), the handlers we're adding here wouldn't actually be used outside the Notification module. I doubt that it makes sense to add these listeners just for #2419 - would be simpler to just trigger the show listener directly there.

comment:13 Changed 4 years ago by kzar

  • Owner set to kzar

comment:14 Changed 4 years ago by kzar

  • Owner kzar deleted

comment:15 Changed 4 years ago by fhd

  • Blocking 2419 removed

comment:16 Changed 2 years ago by sebastian

  • Tester set to Unknown

This sounds sensible, and meanwhile #2419 moved forward without this change, anyway. So it seems this issue should be rejected now, but that is of course up to the module owner/peers.

Last edited 2 years ago by sebastian (previous) (diff)

comment:17 Changed 20 months ago by fhd

  • Cc trev removed

comment:18 Changed 3 months ago by alicetaylor

Thank you for sharing valuable information. Nice post. I enjoyed reading this post.

http://returnman3game.net

comment:19 Changed 8 weeks ago by phamyen

Thanks for sharing such a nice piece of information to us.
http://pogogamesplay.com/

comment:20 Changed 7 weeks ago by redball12345671211

Your contributions to make it possible to register a notification download listener deserves to be noted.
http://raze3.co/strike-force-heroes-2

comment:21 Changed 2 days ago by ducklife

The knowledge you share really changes me in life, I sincerely thank you for what you have done, surely your blog will help more people.
https://templerun.co

Note: See TracTickets for help on using tickets.