-
Notifications
You must be signed in to change notification settings - Fork 228
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
Allow adjusting the fontsize of the pulsectl default device popup #986
Allow adjusting the fontsize of the pulsectl default device popup #986
Conversation
Nice addition, thank you very much! Great to read from you again! I think eventually, the best option probably would be having this as global (module independent) parameter. What do you hink? |
Oh, I didn't know you can have global ones. That would definitely be preferred then :D How do I define it globally? |
Those are the parameters defined outside the modules (in config.py, i think, can't remember for sure and don't have a pc handy) I think popup should then have access to that via a config object. |
Looking at it in a bit more detail:
If you prefer, I can put together a draft PR for those changes, as they are targeting the core quite a bit. |
Sorry, I haven't had much time these days to finish this. I've removed the code from the pulsectl module and I agree with your third point that it should ideally all be in config and the popup module. The issue I'm having is that config is a Class definition, but the instance is only available in the main file. Would it make sense to have config be a singleton that can be imported from other modules? |
An instance (the instance, actually) of Config is passed as the first parameter to init() of all modules, iirc. Does that help? |
59587b4
to
19e3032
Compare
@@ -75,19 +75,15 @@ def update(self): | |||
|
|||
def popup(self, widget): | |||
"""Show a popup menu.""" | |||
menu = util.popup.PopupMenu() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to have been using an old implementation, so I've updated it
@@ -77,7 +77,7 @@ def popup(self, widget): | |||
util.cli.execute(popupcmd) | |||
return | |||
|
|||
menu = util.popup.menu() | |||
menu = util.popup.menu(self.__config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the best I came up with for passing on the config object to the popup menu. WDYT?
19e3032
to
0a3c6fe
Compare
0a3c6fe
to
37b5646
Compare
Looks good, thank you very much! |
I've recently started using the pulsectl tk popup to pick the default device and one annoying thing is that the default font size is way too small for me, so I added basic support for changing it. I'm not sure if this is a good way to implement it, let me know if not :)
Edit: I just remembered that I should probably also update the docs 😅 I'll try to get around to that later today after work