-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
Add show-icon setting for keyboard-layout applet #466
base: main
Are you sure you want to change the base?
Conversation
Code looks good at a glance (especially for a first attempt!), but will need to clone and properly test this. |
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.
It works perfectly as far as I can tell, but the settings page doesn't look right - looking at the XML, it doesn't use the same widgets/margins as the other built-in applets' settings pages. I'd recommend copying another applet's settings.ui, like the tray applet, and just replace the labels/controls with the one you've implemented. Just for consistency's sake, that's all.
First of all, thank you for testing the code and notifying me about the inconsistencies. I've changed the ui file based on the tray applet as you've suggested and I've also compared it to some other applets (like the clock applet for example). After recompiling and testing the code, the margins seem to be correct and consistent with the other applets. Please let me know if there's anything else that needs to be done! |
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.
LGTM. Did some cleanup of the spacing on the applet layout children while I was pixel peeping. Will be consistent now.
Settings section looked good too.
This will ensure spacing between widgets is consistent regardless of layout and we don't have to do anything strange with margin start / ends
Description
This pull request adds an option to show/hide the icon of the keyboard-layout applet. I think it's a nice option to have and it is especially useful for smaller screens, where we need all the space we can get for other applets on the panel.
Additionally, I also noticed that the icon for the applet was a bit off in a vertical panel, therefore I've changed the code that handles the margins based on the orientation.
Keep in mind that I've only started using Budgie a week ago and I'm not very familiar with the code. I used the other applets as a reference to implement this feature. If there's anything wrong with the code I wrote, please let me know, I'd love to learn more and improve!
Submitter Checklist
git rebase -i
(if needed)