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

Pressing a notification just opens the openHAB app #379

Closed
FlorianSW opened this issue Sep 24, 2017 · 8 comments
Closed

Pressing a notification just opens the openHAB app #379

FlorianSW opened this issue Sep 24, 2017 · 8 comments

Comments

@FlorianSW
Copy link
Member

When a notification arrives, it will create a notification element in the notification bar with the message (which is also stripped when it's too long). When the user presses this notification, the only action taken is to open the openHAB app.

Instead of doing that, the openHAB app should be opened with the Notification tab opened by default, or the message should be visible as a full text string (e.g. in an "OK-able" message window) when the notifications tab is not available, e.g. because you're connected to the local network, see also #253).

FlorianSW added a commit to FlorianSW/openhab.android that referenced this issue Sep 24, 2017
If a notification from the systems notification bar is selected, the app
now opens the notifications fragment, if it is available currently
(openHAB remote url with username and password is used).

It also shows an alert box with the message content of the selected
message, if the message was added to the intent (should always happen).
This will happen regardless if the notification fragment is opened or not
to show the full message to the user (again).

Fixes openhab#379
@dominicdesu
Copy link
Contributor

Instead of doing that, the openHAB app should be opened with the Notification tab opened by default, or the message should be visible as a full text string (e.g. in an "OK-able" message window) when the notifications tab is not available, e.g. because you're connected to the local network, see also #253).

Actually, the notification tab should always be available, whether openHAB is accessed locally or remotely. In the latest app version, it isn't displayed in both cases (see latest comments #253) for me (does it work for you?).
In my opinion, it is therefore not necessary to show a message window, but instead fix #253 and then always go to the notification bar and open the selected notification. I think the notification bar (when it still existed) was able to show the full text, but I am not sure and I cannot check (because it is missing).

@FlorianSW
Copy link
Member Author

I know that, I opened a pull request to fix #356 (which was broken by me, sorry for that). However, the behaviour before it was broken was always to show the notifications menu only, if the app is connected to the remote URL, not to the local (as the openHAB itself does not save the notifications, or at least does not provide the same API as openhab-cloud). That, however, is another issue, which is tracked in #253 as you mentioned already.

I agree, that whenever the notifications menu is available (also, if we implement it to be visible when connected locally, too), pressing the notification in the status bar should open the notification tab. I just implemented/suggested the fallback for the current case, when openHAB is connected to a local URL.

does it work for you?

After applying the fix of my PR, yes, otherwise no :)

@maniac103
Copy link
Contributor

FWIW, the full text can be shown by applying a BigTextStyle to the notification. Doing that is a good idea in general, I think.

@dominicdesu
Copy link
Contributor

dominicdesu commented Oct 1, 2017

However, the behaviour before it was broken was always to show the notifications menu only, if the app is connected to the remote URL, not to the local (as the openHAB itself does not save the notifications, or at least does not provide the same API as openhab-cloud).

Actually that wasn't the case in the past. Before #251 had been merged, notifications were shown both if locally or remotely connected. This was working because the notification tab was always using the remote URL, even if connected locally. In the above mentioned commit this has been (probably unintentionally) changed, so that the notifications are using the local URL if locally connected (which is not working for the reasons you mentioned).

Therefore, if #253 would be fixed, the notification bar is always shown and there is no need to show a separate message in my opinion, unless I am overlooking something here.

Btw., seems like the notification texts are/were shown completely in the tab as there was a PR to fix this: #226.

@FlorianSW
Copy link
Member Author

@maniac103 It's already a big text style, however, even that one is limited to 5120 characters, so messages that would be longer would be truncated, too.

@dominicdesu You may be right, however, this should really be discussed in the related issue and not here :]

@dominicdesu
Copy link
Contributor

@FlorianSW in my opinion, part of the commit for the current issue:

or the message should be visible as a full text string (e.g. in an "OK-able" message window) when the notifications tab is not available, e.g. because you're connected to the local network, see also #253)

is a workaround for the bug described in #253. Therefore I was just hoping the original issue could be fixed instead of adding this workaround (because I want the notifications bar back 😃). Or is the popup still useful even if #253 is fixed?
Anyway, if it's not that simple to fix #253 the popup is probably a good workaround for now.

@FlorianSW
Copy link
Member Author

The current dialog is only 7 lines of code, and not so complex code, too. Fixing #253 may be more complex (or may not be more complex), however, it would add complexity to this PR, and I would like to keep it as isolated as possible and fix the issue described in #253 in another PR :)

And, the dialog has the benfit, that the user gets a dialog box with the full message text of the message he selected in the status bar, so he does not need to search for it in the list of the last notifications, which may or may not already been the 3rd or 4th in the list :) So I would call it useful after #253 is fixed, yes :)

@dominicdesu
Copy link
Contributor

Sounds great, thanks!

digitaldan pushed a commit that referenced this issue Oct 8, 2017
If a notification from the systems notification bar is selected, the app
now opens the notifications fragment, if it is available currently
(openHAB remote url with username and password is used).

It also shows an alert box with the message content of the selected
message, if the message was added to the intent (should always happen).
This will happen regardless if the notification fragment is opened or not
to show the full message to the user (again).

Fixes #379
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

No branches or pull requests

4 participants