manisandro / gImageReader

A Gtk/Qt front-end to tesseract-ocr.
GNU General Public License v3.0
1.6k stars 188 forks source link

Build HOCR shortcut dialog with QFormLayout #577

Closed ferdnyc closed 2 years ago

ferdnyc commented 2 years ago

This does only the very, very basic work of replacing the HTML-table-constructed "Keyboard Shortcuts" dialog with one based on QFormLayout, fixing the dialog sizing/layout.

The new code splits apart the keystrokes and descriptions, using QKeySequence::toString(QKeySequence::NativeText) to format the keystrokes themselves. Each description is now a separate translatable string (that's then inserted as a QLabel alongside the QKeySequence::toString() text), which should improve life for translators.

(QKeySequence::NativeText enables auto-translation of key names on all platforms, so those no longer need to be translated at all. Plus, on macOS — if the documentation is to be believed (I don't have a Mac to test it on) — it uses symbol-based formatting for the modifier keys, like you'd see next to a menu option if it has a shortcut.)

Because QKeySequence is incompatible with "merged" keystroke labels such as "Ctrl+{Up,Down}", I had to split those pairings into two separate keystrokes with identical descriptions. I feel the slightly less compact dialog is a worthwhile tradeoff, when stacked against the benefits from using Qt's auto-translations. Plus, I expect that redundancy to be temporary, as there is room for improvement in the current key bindings.

No change to any bindings, mappings, or descriptions is included here, that's for future PRs to tackle.

Partially addresses #570

Screenshots

Default English locale

Screenshot from 2022-02-23 08-20-02

Executed as LANG=de_DE gimagereader-qt6

Screenshot from 2022-02-23 08-22-02

manisandro commented 2 years ago

Thanks! Regarding duplicate keybinding descriptions, actually I guess they could be de-duplicated by writing them like

Adjust left bounding box edge to the left
Adjust left bounding box edge to the right

which would make them also easier to understand.

ferdnyc commented 2 years ago

@manisandro

Well, actually, I was going to suggest a more radical departure. I suppose I might as well just lay it out here.

IMHO, rather than having to remember what combinations of keys do what, it would be simpler if there were one set of bindings to move the bounding box, and a second set to adjust the size of the bounding box (with the upper left corner being fixed).

IOW:

Yes, certain things would take two keypresses where they used to only take one. (Though move adjustments, which used to require two presses, would now only take one.) But on the other hand you never have to try to remember which side Ctrl+Shift adjusts.

manisandro commented 2 years ago

No strong opinions regarding keybindings on my part, so I'd say go for it!

ferdnyc commented 2 years ago

I'll make the necessary adjustments!

Oh, BTW, I also already have the new shortcuts list (the old-new version, like in my screeenshots) processed into a GtkShortcutWindow-based .ui file, for the other side of the world.

(Amazingly, Gtk and Qt can't even agree on how to format key combinations.

So it took a fair amount of munging.)

manisandro commented 2 years ago

Thanks for taking care of Gtk also!

(Sometimes it's an interesting exercise to stay up-to-date with both toolkits, sometimes I wonder whether it's really worthwhile...)

ferdnyc commented 2 years ago

Oh, the Gtk stuff is definitely a long way from anywhere; all I have is an un-integrated UI file. And since Glade doesn't even support GtkShortcutWindow, there's no way to preview or examine it without integrating it into the code first. Which I haven't even touched.

(Though I did create a branch ferdnyc:gtk-proofread in my fork, where I applied the contents of the gtk/src/hocr/wip_proofreadwidget/ directory, since keeping that stuff on a topic branch instead just seemed like a win to me. If you want to create a branch in the project repo for it, I can open a PR targeting it and push that stuff here. The (updated) ShortcutsDialog.ui file is also on that branch.)

Anyway, the commit I just pushed here implements the new keybindings in the Qt ui. Obligatory updated screenshot:

Screenshot from 2022-02-23 17-39-45

manisandro commented 2 years ago

Thanks!