zagortenay333 / cronomix

All-in-one timer, stopwatch, pomodoro, alarm, todo and time tracker gnome-shell extension
MIT License
580 stars 75 forks source link

Fix extension settings are crashing #168

Closed jul13579 closed 3 years ago

jul13579 commented 3 years ago

This PR fixes crashing of the extension settings.

I want to make exceptionally clear, that I did not yet use the extension. I just stumbled across it today and thought it was neat but had this issue to be addressed. Hence, I do not know if every feature still works as intended. Also, this is my first attempt at writing/contributing to Gnome extensions. Any help/improvements/tips are highly appreciated.

Closes #167

zagortenay333 commented 3 years ago

Thanks a lot. A couple of points:

jul13579 commented 3 years ago

Thanks for the feedback!

  • It seems that the entries are not responsive. Those for setting the keyboard shortcuts.
  • Toggle buttons that sit right next to normal buttons are vertically stretched.

Will look into both :+1:

  • What is that BuilderScope thing you added?

https://gjs.guide/extensions/upgrading/gnome-shell-40.html#gtkbuilder (Somehow this link won't correctly jump to the anchor on my machine. The info is located at Prefs > Template Classes > GtkBuilder) As far as I understood, the old way of connecting a signal is no longer possible and was replaced by either registering custom widget classes that implement the click functions, or by adding this BuilderScope implementation that implements the click functions. I thought the BuilderScope was less of an intrusive solution to the already existing implementation, so I chose it. Additional Note: Since I decided to use the same click function for all buttons, it required that I somehow pass the settings key that holds the filechooser-path to the builderscope. I tried to use GObject.set_data and get_data respectively, but I didn't get it to work (or maybe I am misunderstanding its purpose). That's why I also introduced this map that maps the button IDs to the corresponding settings keys.

I try to get back with updates soon!

Edit: Apperently I misunderstood the porting guide regarding this BuilderScope solution. I tested manually connecting the signals (as it was previously) and it is still working. I think I got confused during porting of the old FileChooser to the new FileChooserNative. Hence, I will try to remove BuilderScope and connect the signals manually.

jul13579 commented 3 years ago

Updated the PR :wink: I think this should solve your mentioned problems. I also discovered a change in Gtk.accelerator_parse which broke validation of keybindings.

zagortenay333 commented 3 years ago

Awesome! Thanks again.