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

Notifications #559

Closed
mario opened this issue Jan 19, 2017 · 43 comments
Closed

Notifications #559

mario opened this issue Jan 19, 2017 · 43 comments

Comments

@mario
Copy link
Contributor

mario commented Jan 19, 2017

Notifications are an important part of a workflow, so I want to implementation notifications screen.
Pulling would be every 5 minutes if there are apps that can generate notifications otherwise we can pull every 2 hours or so. This can, of course, be tweaked later on.

Since we want the user to receive notifications even when app is inactive, I assume we need a service implementation, but @tobiasKaminsky and @AndyScherzinger can probably clarify that.

Clicking on a notification while the app is inactive would open the app and the notifications screen for the user that got the notification. This would also trigger some kind of message that "User Mario Danic logged in" or something similar coming up at the top of the screen.

Involving @nextcloud/designers so they can tell me how wrong I am on the overall workflow and so they can deliver mockups/workflows I'd work against.

Involving @karlitschek to tell me more about the vision behind the functionality, and involving @nickvergessen because he seems to be the chief of all things API.

@jancborchardt
Copy link
Member

It should look exactly like the Activity app on the server. :) That’s your live mockup basically.

@mario
Copy link
Contributor Author

mario commented Jan 19, 2017

@jancborchardt understood. The other issue mentions merging uploads, notifications and activities though. (And even on the server, notifications & activities are different)

@jancborchardt
Copy link
Member

Basically notifications are a more urgent and often interactive version of an activity. Like someone calls you, you have to pick up. Or an app needs to be updated etc, someone shares a file with you etc.

They are all chronological information. If you need to act upon it (upload error, sync error, you need to accept a share, you need to update something, or pick up a call) they should be shown on the top of the list.

@mario
Copy link
Contributor Author

mario commented Jan 19, 2017

@jancborchardt understood. Thanks for the clarification.

@nickvergessen
Copy link
Member

Notifications have nothing to do with activities, please ignore that.
Notifications are not chronicle but always urgent and they should also be shown as android notifications. Not hidden in any view (also thinking about multiple accounts on one device), that can be done additionally, but not primarily. Notifications spec matches the HTML notifications spec for exactly this reason. So they can be directly piped into the OS notifications.

This was very much misunderstood on the desktop client.

bildschirmfoto vom 2017-01-19 15-43-11

Basically the notification says:

Notification - Action required
You received one new notification on cloud.nextcloud.com

So you have to check the client in the non default tab on a second level navigation, 4 clicks away...:

bildschirmfoto vom 2017-01-19 15-45-15

And there the domain is even missing, so you don't know which server this message is from.

Instead the actual text "Frank K. invited you to a call" should be the actual notification text. And the Account/Instance should also be listed as detail.

Hope this helps to clarify things.

@eppfel
Copy link
Member

eppfel commented Jan 20, 2017

@nickvergessen Although notifications have to be handled differently, than activities, it is questionable to give them their own View in the app or even a sidebar item.
In #556 we discuss to combine the notifications into the activity stream. I actually think, when we integrate the notifications in Android natively, is there still the need to show them in any view inside the App? Choosing to act on the notification should directly transfer to it's content: Incoming call: open spreed.me, Update available: open admin settings, etc.

@nickvergessen
Copy link
Member

Yeah, that's what I ment. It should be in the notification itself, and then you don't need to combine it with anything. I just wanted to point out, that the current desktop implementation is a very bad example.

@tobiasKaminsky
Copy link
Member

As an example "Incoming call: open spreed.me". Does the notification has all the informations that our app can handle the notification correctly? Or do we have to "parse" every notification and handle them individually?

@mario
Copy link
Contributor Author

mario commented Jan 20, 2017

What worries me is ... since we have to pull, things like incoming call won't really work as expected (worst case, you'll get notification 5 minutes after someone called you).

@nickvergessen
Copy link
Member

Yeah don't be worried, we are discussing this atm.

@mario
Copy link
Contributor Author

mario commented Jan 23, 2017

Keep in mind that even if we don't have a Notifications view per-se, we will need to create one that will be triggered by the notification click on Android 4.0 since it does not supports notification action buttons as far as I remember. Obviously, it's not a menu item in that case :)

@eppfel
Copy link
Member

eppfel commented Jan 23, 2017

Keep in mind that even if we don't have a Notifications view per-se, we will need to create one that will be triggered by the notification click on Android 4.0 since it does not supports notification action buttons as far as I remember. Obviously, it's not a menu item in that case :)

Is there something like an redirect view?

@mario
Copy link
Contributor Author

mario commented Jan 23, 2017

@eppfel redirect view? Clicking on the notification will launch a view/activity that we define and create. It should have a text field and a place where we can put an arbitrary number of buttons depending on the data.

@nickvergessen I assume the number of actions that could be attached to an action can be more than 2 (examples mention only 2)?

@nickvergessen
Copy link
Member

Yeah, there is no hardcoded limit, current known implementations only use 0, 1 or 2, but in theory there is no limit

@eppfel
Copy link
Member

eppfel commented Jan 23, 2017

@mario Ah, ok. You are right.

@jancborchardt
Copy link
Member

Mario and I discussed about this, see #557 (comment)

In the desktop client the mistake was made to do a separation between 3 chronological streams (server activity, uploads, and errors) which are basically the same, or contain the same items.

This is similar for notifications, as many of them (like accepted shares, or comments you could reply to) will show up in activity too. It’s a very similar problem as: Sidebar: combine file Activity, Comments and Versions into unified »Activity« timeline tab in nextcloud/server#658 – that leads to people not discovering stuff, things being duplicated (Versions and Comments show up in Activity but are not interactive there …) and the whole app seeming more complicated than it really is.

@MorrisJobke
Copy link
Member

@mario @nickvergessen Was there already work done for Android? (during the Hackweek)

@mario
Copy link
Contributor Author

mario commented Feb 28, 2017 via email

@mario
Copy link
Contributor Author

mario commented Feb 28, 2017 via email

@MorrisJobke
Copy link
Member

Just to note it down here as well:

The API is currently developed in nextcloud/notifications#59 and the push proxy can be found at https://push-notifications.nextcloud.com (ask @LukasReschke or @nickvergessen for access and help)

@aleister09
Copy link
Contributor

I'm interested in to develop the Notification Feature
How can i start?
Do you have a mockup about this feature?

@MorrisJobke
Copy link
Member

Do you have a mockup about this feature?

@mario @jancborchardt I guess you looked into this during the last hackweek.

@nickvergessen I installed the notifications app from the branch in nextcloud/notifications#59. Is anything else needed?

cc @oparoz FYI

@MorrisJobke
Copy link
Member

@mario @jancborchardt I guess you looked into this during the last hackweek.

There it is #557 (comment)

So as there is currently no activities UI available I guess it's fine to have for now a separate notifications panel, that then can get merged into the activities view once that one is there.

Right @AndyScherzinger @tobiasKaminsky ?

@nickvergessen
Copy link
Member

Just for the record as far as I know we don't push to android, but pull there.
Because we don't want to depend on google play services (for the fdroid store)?

@mario
Copy link
Contributor Author

mario commented Mar 25, 2017

@nickvergessen we don't do push for the initial implementation, indeed - but we already rely on GMS for other things unfortunately :-/

@aleister09 @AndyScherzinger already started working on models and UI for both notifications and activities I guess - but let's see if he wants to hand you one of those, and once he decides which, I'll give you all the info you need (where to put models, api calls, UI stuff, DB stuff, and others).
@AndyScherzinger your turn to decide if you prefer notifications or activity :D

@AndyScherzinger
Copy link
Member

Hi all, haven't found the time to start working on it, so pick whatever you fancy @aleister09. For activities we already talked about how the UI should look like so that one is already quite defined and I would suggest you pick that one. From the discussion with @mario the first step would be to extend the library project for Api connection and data model, then extend the app project to utilize this, add a new menu item, activity with recyclerview :)

@AndyScherzinger
Copy link
Member

@MorrisJobke we also have this #557 (comment)
Which is the result of the discussion between Jan and me on how the items should look like. For the date human readable we already have a function used for the file list we can also use here.

@AndyScherzinger
Copy link
Member

@AndyScherzinger
Copy link
Member

@aleister09 and @mario I already added the menu (+icon) item and an empty activity to the branch for you, same for notifications

@AndyScherzinger
Copy link
Member

empty notification screen is done (need to rewrite this for activities too):
device-2017-03-27-191007

@AndyScherzinger
Copy link
Member

Work in progress PR has been opened: #783

@AndyScherzinger AndyScherzinger added this to the Nextcloud App 1.5.0 milestone Apr 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants