lanoxx / tilda

A Gtk based drop down terminal for Linux and Unix
GNU General Public License v2.0
1.28k stars 161 forks source link

Revamped Keybindings Page #234

Closed pik closed 7 years ago

pik commented 8 years ago

This is a cleaned out version of @x2b keybinding branch commit, I've gotten rid of all the unnecessary tilda.ui changes, but left his code changes in wizard.c mostly as is (except for conflicts since update). There are a few parts which I think could still be improved where I've left comments. #163

lanoxx commented 8 years ago

Looks mostly good. I agree with your comments in commit 417a39b but I do not have time to cleanup the patches. If you want to do that let me know. Otherwise around June I might have some more time then I will see to get this merged.

Two more things:

  1. I think I would prefer if the key changes could be activated with a single click, right now it requires a double click.
  2. There should be some visual clue when the mouse hovers over the shortcuts column to indicate that these cells can be activated. Maybe highlight the shortcut cell on hover?
lanoxx commented 7 years ago

I finally took the time to clean this up and merge the code. Most notably is that I followed @pik's suggestion and moved all the keybinding code into a separate tilda-keybinding.c/h set of files. I also removed the global list store variable in that process and I slightly changed the loop that sets up the tab keybindings. This should fix all the things that @pik suggested in his comments.

I still think some more visual feedback would be nice but we can handle this in another issue.

@pik @x2b Thanks to both of you for your amazing work and sorry this took so long to merge. Please test this if you can and tell me if if you can find any issues that I missed.