fds2610 / NCT-Desktop

Desktop-App for notifying Nextcloud-Talk Events
Creative Commons Zero v1.0 Universal
37 stars 3 forks source link

[Enhancement] Open Spreed url by default if no notification #18

Closed Baccanno closed 4 years ago

Baccanno commented 4 years ago

https://github.com/fds2610/NCT-Desktop/blob/eeb19cd935ca932ddc292516d73290edd4670455/nct.js#L70 Shouldn't the mLink default url be NC talkURL ? for now it opens base NC page, and you have to click on the talk button.

fds2610 commented 4 years ago

Hi @Baccanno , yes and no. mLink takes Base-URL as a default as long as we received no notification. As soon as the notification is received, mLink gets updated and will keep directing us to Talk (last conversation). I COULD look for some Talk-URL and set that in future... :-)

fds2610 commented 4 years ago

how about just appending "/index.php/apps/spreed/" as a default?

Baccanno commented 4 years ago

I believe it is yes

Baccanno commented 4 years ago

Hi @Baccanno , yes and no. mLink takes Base-URL as a default as long as we received no notification. As soon as the notification is received, mLink gets updated and will keep directing us to Talk (last conversation). I COULD look for some Talk-URL and set that in future... :-)

Yes I was indeed talking about the state where there is no notification. It would actually even be better if it could open the "last" notified conversation, or the first in the list. All this could be optional ticked boxes. But there is no drawback to open the last notified conversation in NC talk as the UI still shows the other ones.

I do not have knowledge upon the Talk API so I don't now what can be done.

fds2610 commented 4 years ago

well, this is not so complicated: a) the base-url is fixated in the config-input-field. b) with CURRENT release of nextcloud to me it seems that Talk can be reached pretty sure by adding "/index.php/apps/spreed/" to the base url c) with every Talk-notification i receive an individual link directly to that message in that conversation

as I understand you, you would rather jump to b) at any time than using c) as soon as it was set or make it a configurable behavior ?

Baccanno commented 4 years ago

Yes that's quite that

I believe it should be the default behaviour as it does not add more clicks if you want to do another action. So no need to make it configurable for now I guess.

fds2610 commented 4 years ago

@Baccanno do you use my sourcecode or do you need a "compiled" zip to test ? I added this change to the branch new-login-flow but did not create a zip file, yet.

Baccanno commented 4 years ago

I'm using the compiled version having no electron setup locally. I might do it in the future. Would be quite interesting for everybody, yourself included, to have a CI running in order to always have the last built test automatically available.

Baccanno commented 4 years ago

I actually found this repo that seems to make the process painless ! https://github.com/samuelmeuli/action-electron-builder

It seems you'll just have to tag your releases like this "git tag 1.0.X RCX && git push --tags" and it will automatically create a build.

I'll put that in another issue if you feel like it.

fds2610 commented 4 years ago

@Baccanno thanks for your contribution which i really appreciate! currently i try to understand github&git so it is ok for me to compile everything manually, but I will keep your hint in mind. maybe we should open a separate issue for that! in the meanwhile i compiled a version for you: http://code.schepmann.de/NCT-Desktop/nct-desktop-win32-ia32-Baccanno.zip :-)

fds2610 commented 4 years ago

well i implemented this issue in 0.1.1 Beta ( #19 branch) , please test. all binaires are uploaded and ready for testing.

Baccanno commented 4 years ago

The configuration work as expected but the wording is misleading : "default to base-url instead of NC-Talk directly" should be "Default to NC-Talk instead of base URL" (When checked it actually redirects to NC-Talk)

Thanks for the enhancement is already much more user friendly.

Baccanno commented 4 years ago
  • I'd like to jump to C (last notified message) whenever I can.
  • I could jump to B as a fallback. But I'd rather jump to my first stared conversation or "Last active"

Tell me if you need other feedback for the follow up. I wasn't able to test notifcation locally with a 2nd account and private navigation. No notification are received. I wonder why

Baccanno commented 4 years ago

I can confirm that notification are not showing up anymore on the latest build.

fds2610 commented 4 years ago

d*mn... it is always this last little hot fix one implements after Q and regression tests... i updated& uploaded all binaires.... pls re-test

Baccanno commented 4 years ago

Okay, last test I did was good :

Great additions 👍

fds2610 commented 4 years ago

as this and issue #17 seem to work now. i close both and merge it all, now. Thanks for testing @all !