Merge lp:~mr-fiorotto/wingpanel-indicator-notifications/trying-to-fix-1487291 into lp:~wingpanel-devs/wingpanel-indicator-notifications/wingpanel-indicator-notifications
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Adam Bieńkowski on 2017-03-01 | ||||
Approved revision: | 136 | ||||
Merged at revision: | 147 | ||||
Proposed branch: | lp:~mr-fiorotto/wingpanel-indicator-notifications/trying-to-fix-1487291 | ||||
Merge into: | lp:~wingpanel-devs/wingpanel-indicator-notifications/wingpanel-indicator-notifications | ||||
Diff against target: |
13 lines (+2/-1) 1 file modified
src/Services/Notification.vala (+2/-1) |
||||
To merge this branch: | bzr merge lp:~mr-fiorotto/wingpanel-indicator-notifications/trying-to-fix-1487291 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Adam Bieńkowski | code / testing | 2017-01-19 | Approve on 2017-03-01 |
Review via email:
|
Commit message
Don't save "transient" notifications
Description of the change
Check if the transient parameter is in the hints Variant, if true don't add the notification to the list.
- 135. By Giuliano Fiorotto on 2017-01-19
-
changed check position to get_is_valid #1487291
Giuliano Fiorotto (mr-fiorotto) wrote : | # |
> You should be using that check in get_valid method in Notification class
> instead of using it in add_entry.
I've moved that check in get_is_valid method in Notification class.
Adam Bieńkowski (donadigo) wrote : | # |
Though, this code works, you are interpreting the transient value wrong. Currently you are checking if it only *exists*, while you should be checking if the "transient" value is false.
What I mean is lookup returns if the key exists not it's actual value e.g: you can execute a "Notify" method in d-feet to see how the indicator behaves with these parameters:
"AppName", 0, "dialog-
You can see even if we sent "False" your solution will mark the notification invalid.
Please use "lookup_value" and "get_boolean":
var transient = hints.lookup_value ("transient", VariantType.
return ... && (transient == null || !transient.
- 136. By Giuliano Fiorotto on 2017-02-05
-
fixed code accordig to @donadigo suggestion
Giuliano Fiorotto (mr-fiorotto) wrote : | # |
We just checked if the transient field exist because in our tests if there was it was true, but of course it was wrong. Thanks for the d-feet suggestion :D
Adam Bieńkowski (donadigo) wrote : | # |
Thanks for your work!
Looks good now.
You should be using that check in get_valid method in Notification class instead of using it in add_entry.