-
Notifications
You must be signed in to change notification settings - Fork 10
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
Rewrite to use new QuickSettings API in Gnome43 #8
base: main
Are you sure you want to change the base?
Conversation
The aggregateMenu which was previously in use is no longer supported: https://gjs.guide/extensions/upgrading/gnome-shell-43.html#quick-settings
if (error) throw error; | ||
// proxy.connectSignal('BrightnessChanged', this._sync.bind(this)); | ||
// this._sync(); | ||
}); | ||
} |
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 main part I'm unsure about. Can you let me know what the purpose of it is? In general this sync function but also the lastChange calculation.
I personally run Gnome 42, so I cannot test this. If this is a complete rewrite, and the user interface is different too, maybe you could publish it as a separate extension ? You can add me as a contributor, and I'll contribute when I upgrade to gnome 43. |
The sync and lastChange code allows for a smooth experience when changing the keyboard brightness using the slider. This is particularly useful when the hardware only supports a limited number of brightness levels (for example, my keyboard has only three levels). Instead of immediately updating the brightness to the exact value of the slider, we wait a few milliseconds to see if the user has stopped sliding. If the user has stopped moving the slider, we round the value to the nearest supported brightness level. This helps prevent the slider from janking or jumping around as it is being moved. |
Thanks for the info on the sync function. Here is what it looks like, for reference: I think the end result is pretty much the same, if I can re-incorporate the sync function then the behavior should be the same. I wanted to avoid creating more extensions on the extensions store thing because there are already way too many, half of which don't work. I guess it depends if you want to maintain the ability to provide fixes/updates for |
It is not an overkill anymore, newer GNOME (>=45) versions have this built-in, so this extension would only make sense if it were for older versions. |
The aggregateMenu which was previously in use is no longer supported:
https://gjs.guide/extensions/upgrading/gnome-shell-43.html#quick-settings
Fixes #7
@lovasoa I could use your help to get this PR fixed up to be mergeable. To be honest I have no idea what I am doing; the documentation on gnome-shell extension development seems pretty thin on the ground and you have to connect a lot of dots yourself (there is no complete example). The last time I wrote any JavaScript, jQuery was the latest and greatest! 😅
I basically started from scratch so a lot of stuff has gone missing (the
_sync()
function etc) and I have no clue what any of it does. This plugin works on my machine however! Perhaps some of it is not needed with the new QuickSettings API?Let me know what you think and how we should proceed.
Thanks,
Aaron