Show Appicon in notification #106
Conversation
jmattheis
left a comment
jmattheis
left a comment
There was a problem hiding this comment.
Hey @newhinton,
Thanks for your first contribution! I've added some suggestions for improvements in sub comments.
app/src/main/java/com/github/gotify/service/WebSocketService.java
Outdated
Show resolved
Hide resolved
app/src/main/java/com/github/gotify/service/WebSocketService.java
Outdated
Show resolved
Hide resolved
| try { | ||
| request= cache.load(Utils.resolveAbsoluteUrl(settings.url() + "/", appIdMap.get(appid))); | ||
| icon = request.get(); | ||
| } catch (IOException e) { |
There was a problem hiding this comment.
Log the exception with com.github.gotify.log.Log#e(java.lang.String, java.lang.Throwable)
app/src/main/java/com/github/gotify/service/WebSocketService.java
Outdated
Show resolved
Hide resolved
app/src/main/java/com/github/gotify/service/WebSocketService.java
Outdated
Show resolved
Hide resolved
app/src/main/java/com/github/gotify/service/WebSocketService.java
Outdated
Show resolved
Hide resolved
| import java.util.Map; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.concurrent.atomic.AtomicInteger; | ||
|
|
There was a problem hiding this comment.
The travis-ci build currently fails because of formatting errors. Run ./gradlew spotlessApply to fix them.
https://travis-ci.org/github/gotify/android/builds/677337457#L1868
app/src/main/java/com/github/gotify/messages/MessagesActivity.java
Outdated
Show resolved
Hide resolved
|
I''ll just comment here, thanks for your feedback! I will change the code, i didn't now some of the stuff you pointed out. |
|
Inactivity. |
|
@jmattheis Sorry for the inactivity, i have updated my pr. Would you still check it out? Should i make a new pr or can you reopen this one? I made the mistake and commited a file with my default fileheader wich contains my default GPLv3. I have changed that to MIT, is that enough? |
|
Yeah sure, you can re-open this PR.
Should be ok I guess. |
|
I dont think that i can reopen it, since i am not a member of this project |
jmattheis
left a comment
jmattheis
left a comment
There was a problem hiding this comment.
Could you resolve the merge conflicts?
app/src/main/java/com/github/gotify/messages/MessagesActivity.java
Outdated
Show resolved
Hide resolved
|
I have made changes, i hope i got everything |
|
Yeah, give me some days, I'm currently pretty busy. |
app/src/main/java/com/github/gotify/picasso/PicassoHandler.java
Outdated
Show resolved
Hide resolved
|
Could you rebase your branch? |
2 participants
This adds the appicon to the notification. It adds a little touch to it and makes it look nicer imho. If the appicon is not fetched or some other error occured, the gotify icon is used