Add preferences for optional album info to notifications #681
Conversation
rcalixte
force-pushed
the
notify_album
branch
from
a5f2100
to
a269be5
Compare
rcalixte
force-pushed
the
notify_album
branch
3 times, most recently
from
1a3acf0
to
05e0099
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
rcalixte
force-pushed
the
notify_album
branch
4 times, most recently
from
02533d6
to
b8d7ff4
Compare
rcalixte
force-pushed
the
notify_album
branch
19 times, most recently
from
2c36c2f
to
55b8739
Compare
rcalixte
force-pushed
the
notify_album
branch
from
55b8739
to
332a9cd
Compare
rcalixte
changed the title
|
@TingPing Thank you for the patience. This is now ready for review. |
rcalixte
force-pushed
the
notify_album
branch
from
332a9cd
to
35f7c19
Compare
pithos/plugins/notify.py
Outdated
| self.settings = settings | ||
| self.show_album = self.settings['data'] == 'True' if self.settings['data'] else False | ||
|
|
||
| box = Gtk.Box() |
There was a problem hiding this comment.
It looks very squished together, you should set the margin. Like 6 all around maybe?
There was a problem hiding this comment.
The spacing helped but I just added a pair of new-lines to the label. I think it helps how it looks?
pithos/plugins/notify.py
Outdated
| self.switch.set_valign(Gtk.Align.CENTER) | ||
| box.pack_end(self.switch, False, False, 2) | ||
|
|
||
| content_area = self.get_content_area() |
There was a problem hiding this comment.
This is deprecated, can you just do self.add(box)? I forget if that works if you could test.
There was a problem hiding this comment.
(pithos:2): Gtk-WARNING **: 13:00:25.803: Attempting to add a widget with type GtkBox to a NotifyPluginPrefsDialog, but as a GtkBin subclass a NotifyPluginPrefsDialog can only contain one widget at a time; it already contains a widget of type GtkBox
I'll test with it though.
There was a problem hiding this comment.
The variable naming is different but the latest (assuming it's up to date since it was last edited over two years ago) example suggests the structure is correct.
I'll see if I can find more recent examples.
There was a problem hiding this comment.
I see the deprecation notice with GTK4. I've switched it from a GtkDialog to a GtkWindow object and that seems to be working fine minus one issue where the preference is displayed away from the plugins and main application windows. I'll keep looking to see if I can resolve this.
There was a problem hiding this comment.
I think this is just a byproduct of using Gtk.Window instead of Gtk.Dialog. Window managers will position the Window object the same as other window objects. The other preferences are still using Gtk.Dialog so this will create a bit of inconsistency in terms of user experience.
I'll commit whichever you prefer. If it's Gtk.Window, this is ready to merge as is. If it is Gtk.Dialog, I can push the previous state.
There was a problem hiding this comment.
I thought of another thing. If Gtk.Window is the direction you want things to go while also wanting the user experience to be uniform, I can work on PRs for all of the other plugins with preferences. Then the PRs could be review/merged simultaneously.
rcalixte
force-pushed
the
notify_album
branch
5 times, most recently
from
cb95dc2
to
6afc120
Compare
|
@TingPing Will you have a chance to provide feedback on direction here? I believe this is ready to merge but I can make any edits if needed as well. |
No description provided.