search

sugarlabs / sugar-toolkit-gtk3

Sugar Learning Environment, Activity Toolkit, GTK 3.
GNU Lesser General Public License v2.1
21 stars 80 forks source link

Add ctrl y for RedoButton #445

Closed ryan-van closed 4 years ago

ryan-van commented 4 years ago

Just wanted to add a QOL accelerator for redo button being control + y (this is the usual for many programs).

quozl commented 4 years ago

Thanks. Interesting. By QOL you mean Quality of Life?

I would have thought ctrl+shift+z. See Wikipedia Undo and Ctrl+Y.

I suggest a user experience design discussion on sugar-devel@ may be helpful. Let's get more eyes on this. :eyes:

I've checked none of the activities I care about use either ctrl+shift+z or ctrl+y accelerators. Pippy tutorial 14 uses ctrl+shift+z for undo, but this is not a functional example. I was unable to effectively search the other activities, because GitHub search strips punctuation. I'd have to clone each of them and I don't have the space and funding for that.

Also needs updating on Wiki Hotkeys if merged.

ryan-van commented 4 years ago

Yea, sorry, I did mean Quality of Life.

It seems like Wikipedia is saying most platforms are now incorporating both ctrl+shift+z and ctrl+y. And while this would probably be the most ideal, I quickly looked over the ToolButton code, and it may be too complex and not worth the time to build functionality for both of these actions.

I think from here I can start a discussion on sugar-devel@

Thanks for your input.

quozl commented 4 years ago

Thanks for starting the discussion. It doesn't look like anyone is interested. If we have no answer after a week, I'll choose a design.

Our Write activity embeds AbiWord which has ctrl+y bound to redo. It is not explicit in the activity, but delegated to the AbiWord widget key bindings.

So my choice in the absence of other opinions is to use ctrl+y in order to be consistent with Write activity.

quozl commented 4 years ago

Okay, given there are no other opinions, let's go with ctrl+y, and mention in the commit messages that we chose that to be consistent with Write activity and we didn't get any other opinions.

ryan-van commented 4 years ago

Okay, updated commit and added a comment in the code. Please close the PR if there isn't anything left. Thanks!

quozl commented 4 years ago

Thanks. I've moved the comment into the commit message, as I asked. I've rebased and squashed, then pushed as 72d36cc964de54403af5257c2b2da81c1e83326e.

quozl commented 4 years ago

By the way, I'd normally push changes to your branch, but that doesn't work when your branch is your master. We give guidance already to use a new branch. Sorry if it wasn't obvious why. :grin: