Track Info Dialog: Improve keyboard usability & resize behavior

[cr7pt0gr4ph7]

This PR bundles a number of keyboard usability improvements and general polish for the track info dialog.
It combines a number of small changes that are functionally independent and could be split off into their own PRs if necessary, but would likely produce merge conflicts due to most of them affecting the same set of files.

• In short, the track info dialog and all of its controls (except for the cover art image) are now easily accessible and usable via the keyboard, tab order matches visual order and small screens/large font sizes are handled more gracefully.

• The visual look is only changed minimally (the border style of the comments field now matches the other text fields & the "Import metadata" buttons display an ellipsis when collapsed below their minimum text width & a few pixel-level alignments, see below for details).

PS: If you want to quickly try it out without having to build it yourself, you can use the io.github.cr7pt0gr4ph7.Mixxx Flatpak published at cr7pt0gr4ph7/mixxx-flatpak that includes this and a few other changes. Parallel installation is possible, i.e. any existing official or Flatpak installation of Mixxx will not be affected:

sudo flatpak remote-add --if-not-exists mixxx-flatpak https://cr7pt0gr4ph7.github.io/mixxx-flatpak/default.flatpakrepo
sudo flatpak install io.github.cr7pt0gr4ph7.Mixxx

(As a sidenote, the changes from #13632 have been integrated).

Summary

The Git history has been rewritten to group related commits together into the following changesets (presented in the chronological order of their Git history):

Add "Last Played" field (2 commits)

Because, why not? This was the only piece of metadata that was available as a column but not shown in the track info dialog. There's space in the layout for it, so let's just add it.

Will be shown as - when not present in the track's metadata.

General Cleanup (6 commits)

These should all (hopefully) be uncontentious:

• Fix alignment of the labels for "Comments" and "Grouping" by moving them a few pixels.
• Make text in the "Bitrate" field selectable, like the other read-only fields.
• Just a bit of miscellanceous cleanup:
  • Give the QGroupBoxes speaking names instead of auto-generated ones
  • Use variables to avoid code duplication in updateTrackMetatdataFields
  • Use empty lines to logically group related fields in dlgtrackinfo.h.

Edit star rating via keyboard (5 commits)

The star rating widget was focusable, but it was not possible to use the keyboard to actually change the rating.

• Pressing the 0, 1, ..., 9 key now sets the rating. The result will be clamped to the range 0..maxStarCount (Reason: The existing code internally allows for a maxStarCount other than 5).
• Pressing the + or Right key increments the rating.
• Pressing the - or Left key increments the rating.

As a sidenote, editing the star rating cells in the library table view was suffienctly different to be split into the separate PR #13717.

Improve track list navigation (3 commits)

Navigation within a list of tracks has been improved by adding keyboard shortcuts (Alt+Left/Alt+Right^{1 2}) and conditionally enabling/disabling the prev/next buttons. Focus is kept within the currently focused editor when moving to another track, but any "on focus" behaviours (like selecting all text in a QLineEdit) for the current control are re-triggered.

Future work: It is currently not explained (or visually indicated) to the user that navigating to the previous/next track cancels changes made to the current track (like Escape or Cancel would do).

Resize behaviour (5 commits)

• Avoid the minimum size of WMultiLineTextEdit (the "Comments" field) jumping to a larger size when the dialog is resized by the user.

• Text edit widgets: Show the start of the contained text (e.g. the album title) by default instead of the end when the text's length exceeds the widgets width.

• Allow the Import metadata from MusicBrainz and Re-import metadata from file to be collapsed to smaller widths by displaying an ellipsis (...).

Keyboard navigation using Up/Down (6 commits)

Add Up/Down as an alternative to Tab/Shift+Tab for moving focus within the track info dialog to make the keyboard experience a bit more seamless.

• Exception: For the "Comments" control, focus only moves out of the text editor when the cursor is at the very start or end of the content when you start to hold down the corresponding arrow key, i.e. when you are at the start of the "Comments" field and hold down Down to move the cursor to the very end, focus does not leave the "Comments" field until you release the Down key and then press it again.

• Exception: For the BPM spinner on the second tab, Up and Down retain their original behavior (Note: haven't found a good way to differentiate between the different user intentions here)

Edit color picker via keyboard (3 commits)

The color picker popover could be triggered, but not controlled, via the keyboard.

• Space or Enter on the color picker button triggers the popover and moves focus into the popover.
• Tab/Shift+Tab and Up/Down/Left/Right can be used to navigate within the color picker popover when it is open.
• Pressing Esc closes the color picker popover without making a selection.
• Pressing Enter or Space selects the focused color and closes the color picker popover.

Tooltip for cover art image (1 commit)

Added a compact tooltip to the cover art image to indicate the available interactions (left click/right click).

Merge & additional cleanup (2 commits)

• Merge mixxxdj/mixxx:main and resolve merge conflicts caused by Track Info dialogs: move metatdata buttons below color picker #13632.
• Fix small misspelling meatdata_buttons_layout -> metadata_buttons_layout
• Fix code style issues in wcolorpicker.cpp

Footnotes

1. Sidenote: Alt+Left/Right was choosen because Ctrl+Left/Right and Left/Right already have a meaning within the text edit widgets. ↩

2. Sidenote: Alt+Up/Down can be added if desired. ↩

[github-actions]

This PR is marked as stale because it has been open 90 days with no activity.

[cr7pt0gr4ph7]

@ronso0

Just tried this branch and navigation feels nice indeed! I wonder if the Down/Up navigation will discovered by users since it's rather unusual (at least I didn't see it in other apps, yet). I guess all we can do is document it in the manual.

Spreadsheets applications implement a similar behavior, so IMO this shouldn't confuse users too much. It's also already implemented in Qt for QPushButton, and mostly intended for power users that are used to other data entry applications.

re Down/Up for moving focus: There's an inconsistency now between the single and multi-track dialog now since pressing on the multi-track fields (QComboBoxes) chnages the selected item. Not sure if we should override/disable that and let users open the drop-down with Ctrl+Space like in WSearchLineEdit.

I think that the differing behavior for QComboBoxes is acceptable here, as they serve a different purpose and are visually distinct enough from normal text boxes for users to quickly learn the difference.

[cr7pt0gr4ph7]

@ronso0

The color button now is two lines tall for me (Qt 6.2.3), both in DlgTrackInfo and DlgTrackInfoMulti.

I can reproduce that behavior (the color button gets taller as the dialog gets wider) for DlgTrackInfoMulti, but not for DlgTrackInfo (using Qt 6.7.3). I'll investigate, should probably be an easy fix.

[ronso0]

I came back to this, and right away I remembered why I left last time:
it's too much for my taste. Not necessarily the changeset of 45 commits, I rather feel that adding a WBasic~ class for pretty much every QWidget used in the dialog is overkill. I mean Up/Down to move focus can be almost entirely implemented in DlgTrackInfo. So basically what you implemented in WBasicTabWidget::keyPressEvent.
A minimal POC is here ronso0@f586662

Yes, WMultiLineTextEdit is very helpful for the sizeHint and Up/Down to move focus.
Yes, a WBasicPushButton helps reduce minimum width and gives us a tooltip, just in case.

But that's about it IMHO
Wdyt?

(didn't look into WStarRating in detail, yet)

[ronso0]

some comments..

[ronso0]

What is this for?
just for sake of complete reimplemetation?

[cr7pt0gr4ph7]

WMultiLineTextEdit reuses the rendering primitives from QLineEdit (instead of QPlainTextEdit) to get focus highlighting when focus is inside the control.

The default styles for QLineEdit provide a styling for the read-only state (when QLineEdit::isReadOnly() == true), and as a user, I would expect WMultiLineTextEdit::isReadOnly() to behave similarly.

[ronso0]

Okay, so this is indeed

just for sake of complete reimplemetation

because we don't have read-only multi-lineedits (now).

[ronso0]

Simple if (type == QEvent:Paint) would suffice, less LOC

Or why not override paintEvent()?

[ronso0]

Tbh I'd be more comfortable with QPlainTextEdit::keyPressEvent(event); return; here right after every condition that's not met, even if it's more LOC.
Probably starting with modifiers, then event->key() != Qt::Key_Up && event->key() != Qt::Key_Down, then auto-repeat, then do the fun stuff.

I know all this performs really fast, but still.
Wdyt?

[cr7pt0gr4ph7]

I think both alternatives are equally readable, so I don't have a specific preference. I'll rewrite the code as proposed 👍

(As a sidenote, performance should not be a deciding factor here, as this is in no way a hot path).

[cr7pt0gr4ph7]

I came back to this, and right away I remembered why I left last time: it's too much for my taste. Not necessarily the changeset of 45 commits, I rather feel that adding a WBasic~ class for pretty much every QWidget used in the dialog is overkill. I mean Up/Down to move focus can be almost entirely implemented in DlgTrackInfo. So basically what you implemented in WBasicTabWidget::keyPressEvent. A minimal POC is here ronso0@f586662

IIUC, your concerns are related to WBasicLabel and WBasicLineEdit. I don't really see an advantage in mixing in the focus handling code with unrelated other code in DlgTrackInfo and DlgTrackInfoMulti, instead of cleanly separating it into the two utility classes.

[ronso0]

My point is that my POC is 30 LOC (withou debug stuff), and it's not unrelated, it's isolated in DlgTrackInfo::eventFilter(), theobject which receives the events, ie. easy to maintain. (+ the multi-line edit)
Whereas your proposal is 145 LOC with focus code scattered across 3 extra classes we'd need to maintain.
(WBasicLineEdit, WBasicTabWidget, WBasicLabel?)

edit my new POC 0aa786d also handles the 'no item focused' case, so with that only WMultiLineTextEdit is required for Up/Down to change focus.

[cr7pt0gr4ph7]

My point is that my POC is 30 LOC (withou debug stuff), and it's not unrelated, it's isolated in DlgTrackInfo::eventFilter(), theobject which receives the events, ie. easy to maintain. (+ the multi-line edit) Whereas your proposal is 145 LOC with focus code scattered across 3 extra classes we'd need to maintain. (WBasicLineEdit, WBasicTabWidget, WBasicLabel?)

edit my new POC 0aa786d also handles the 'no item focused' case, so with that only WMultiLineTextEdit is required for Up/Down to change focus.

I see your point. I don't have a very strong opinion on this, so I'll refactor the code as proposed. I'll extend your POC to also handle the labels, though, as these can receive keyboard focus when clicked using the mouse, and for consistency Up and Down should work on them as well.

(For context, my own usage pattern when I don't see which control currently has the focus is to press Up/Down/Tab multiple times until I see the focus frame on some control in my field of view. I therefore prefer if all controls consistently support all suitable navigation patterns).

[ronso0]

Why do the labels need to get keyboard focus?

[cr7pt0gr4ph7]

Why do the labels need to get keyboard focus?

It's the reverse: The labels can get keyboard focus by clicking on them with the mouse (which is necessary to make it possible to copy their contents to the clipboard). They can't currently be focused via the keyboard, but it should be possible to move focus away from them using the keyboard through the same methods as for the other controls.

[ronso0]

The labels can get keyboard focus by clicking on them with the mouse (which is necessary to make it possible to copy their contents to the clipboard)

Hmm this is supposed to be configured by the TextInteractionFlags. But apparently setting those sets the focus policy to ClickFocus. Interesting is that NoFocus set in ui doesn't make a difference (lable can get focus) but if set in c++ the selected text is not cleared when I select text of another label. Qt.. 🤷‍♂️

Anyhow, the build is broken currently due to some } and ; missing. Please fixup and force-push, we'd love every commit to be buildable (to ease later git bisect).

And Down press now jumps twice because of installEventFilter(this);. I removed it and everything still works as expected (even we no widget has focus initially)

Will take a look at the code this weekend I think.