Track Info Dialog: Improve keyboard usability & resize behavior #13818

cr7pt0gr4ph7 · 2024-10-31T17:16:38Z

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¹ ²) 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

Sidenote: Alt+Left/Right was choosen because Ctrl+Left/Right and Left/Right already have a meaning within the text edit widgets. ↩
Sidenote: Alt+Up/Down can be added if desired. ↩

This makes these two labels have the same alignment and pixel position (relative to their input control) as all the other labels in the dialog.

All other similar fields in the dialog are selectable, so this seems to just have been an oversight.

…names

…uts for navigating between tracks

…ifferent track

…rsor position

…zing Also fix the sizePolicy of other widgets, such that the "Comments" field gets all available remaining vertical space.

…ler than contents

…Edit

…TextField

…idget

This is needed to avoid breaking keyboard navigation in a few cases.

The menu is already automatically opened by QPushButton::pressed, no need to do it twice. This caused problems when Enter is used to open the color picker popup.

…ck-info-dialog Resolved conflicts caused by mixxxdj#13632 (Move metatdata buttons below color picker).

ronso0 · 2024-10-31T18:14:22Z

PS: If you want to quickly try it out without having to build it yourself, you can

... just download the CI builds. Info is here https://github.com/mixxxdj/mixxx/wiki/Testing#github-pull-requests

…ith their corresponding edit controls

ronso0 · 2024-11-01T13:31:20Z

Can you please provide before/after screenshots?

cr7pt0gr4ph7 · 2024-11-01T19:43:00Z

As requested, here are before and after screenshots (note: different looking window decorations may be related to Wayland vs. X11):

(Update: The change in the alignment of the "Comments" label is not intentional and will be fixed)
(Update 2: The screenshots were taken before the merge with #13632. Updated screenshots will be provided shortly)

Before	After (at minimum size)

Before	After (at an expanded size)

After: Unfocused comments field	After: Focused comments field

After: Focused star rating

After: Focus rectangle in color picker

ronso0

Thanks for this PR!

Just skimmed through the code and did a shallow first round of review.

ronso0 · 2024-11-01T13:36:58Z

src/library/dlgtrackinfo.cpp

+    m_propertyWidgets.insert("samplerate", txtSamplerate);
+    m_propertyWidgets.insert("replaygain", txtReplayGain);
+    m_propertyWidgets.insert("replaygain_peak", txtReplayGain);
+    m_propertyWidgets.insert("track_locations.location", txtLocation);


The purpose of the QWidget* list is being able to focus editable fields.
What's the benefit of adding the QLabels?

WTrackProperty can be used by skins for arbitrary properties, including the non-editable ones. So, basically, just for completeness sake & to provide uniform behavior for those properties, too.

Hmm AFAIU non-editable fileds (labels) can't be focused anyway, so wouldn't the UI state be somewhat unclear then?
I mean, we wouldn't know the current focus widget/position until we press Tab/Shift+Tab, right?

src/widget/wstarrating.cpp

src/widget/wbasicpushbutton.cpp

ronso0 · 2024-11-01T22:20:20Z

src/widget/wbasicpushbutton.cpp

+    if ((currentSize.height() < fullSize.height() ||
+                currentSize.width() < fullSize.width()) &&
+            !txtLabel.isEmpty()) {


Why do we need to check the height here?

It is possible that there is enough horizontal space but not enough vertical space to display the full button text. The simplest case I can think of is when QPushButton::text() contains linebreaks, but the button height is restricted to a single line.

The fallback tooltip should be enabled in all cases where the button text is not fully visible, which applies here. (Sidenote: It is not enabled when the button text is fully visible to avoid "useless"/confusing tooltips that only duplicate the information shown on the button itself.)

I think we should implement only the cases we need currently and expand as soon as we need multi-line buttons.
IMHO the context should be clear so that we don't need multi-line buttons in the first place.
Or do you have an exmple where we might need one?

src/widget/wbasicpushbutton.cpp

src/widget/wcolorpicker.cpp

Co-authored-by: ronso0 <[email protected]>

cr7pt0gr4ph7 · 2024-11-02T00:27:33Z

Thanks for this PR!

Just skimmed through the code and did a shallow first round of review.

Thank your for the quick review, very much appreciated 👍

cr7pt0gr4ph7 · 2024-11-02T10:30:18Z

src/widget/wbasiclabel.h

+/// to be used like the Tab/Backtab keys.
+///
+/// See WLabel when you need to connect to a ControlObject.
+class WBasicLabel : public QLabel {


@ronso0 I'm thinking whether it make sense to separate the skin widgets and the "basic" widgets into separate folders, e.g src/widget/basic. My gut feeling would be to create a more granular folder structure, though the current practice within Mixxx seems to favor bigger directories + prefixes.

As an aside, one additionally could consider moving the skin widgets into src/widget/skin - though that's a lot of code churn for probably not a lot of benefit.

What are your thoughts on this?

Hmm, I'd rather stick to the widgets directory tbh.

cr7pt0gr4ph7 · 2024-11-02T10:30:36Z

src/widget/wbasiclineedit.h

+
+/// Subclass of QLineEdit that allows the Up and Down arrow keys
+/// to be used like the Tab/Backtab keys.
+class WBasicLineEdit : public QLineEdit {


Same applies here...

cr7pt0gr4ph7 · 2024-11-02T10:30:46Z

src/widget/wbasicpushbutton.h

+/// otherwise overflow the available space.
+///
+/// See WPushButton when you need to connect to a ControlObject.
+class WBasicPushButton : public QPushButton {


...and here...

cr7pt0gr4ph7 · 2024-11-02T10:30:56Z

src/widget/wbasictabwidget.h

+
+/// Subclass of QTabWidget that allows the Up and Down arrow keys
+/// to be used like the Tab/Backtab keys.
+class WBasicTabWidget : public QTabWidget {


cr7pt0gr4ph7 · 2024-11-02T10:31:16Z

src/widget/wmultilinetextedit.h

+
+/// An implementation of QPlainTextTedit with a more sensible minimum
+/// size of 2 text lines.
+class WMultiLineTextEdit : public QPlainTextEdit {


...and here.

ronso0 · 2024-11-03T18:18:35Z

I think the buttons should stretch to full width as before

Previously it looked better with the stars centered below the cover art

ronso0 · 2024-11-03T18:32:54Z

I think the buttons should stretch to full width as before

Oh, they already do. (just the screen shot doesn't reflect that)

ronso0 · 2024-11-03T18:33:48Z

It's nice the star rating can now be focused, but a focus frame would be helpful to make it clear.

ronso0 · 2024-11-03T19:08:30Z

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

ronso0 · 2024-11-03T21:45:54Z

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.

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.

ronso0 · 2024-11-03T21:59:23Z

I'll take a look at this huge change set soonish.

ronso0 · 2024-11-05T15:07:38Z

FWIW I can't seem to shrink the single-track dialog to squeeze the buttons?

ronso0

Some more findings

ronso0 · 2024-11-05T11:22:15Z

src/library/dlgtrackinfo.cpp

+    m_pWCoverArtLabel->setToolTip(
+            tr("Left-click to show larger preview") + "\n" +
+            tr("Right-click for more options"));


Let's move this to WCoverArtLabel so it hasto be translated only once.
Can go to the ctor, protected with if (m_pCoverMenu != nullptr)

ronso0 · 2024-11-05T11:23:37Z

src/library/dlgtrackinfo.ui

               <property name="sizeConstraint">
                <enum>QLayout::SetDefaultConstraint</enum>
               </property>
+               <property name="alignment">
+                <set>Qt::AlignRight|Qt::AlignVCenter</set>


Should be

Suggested change

<set>Qt::AlignRight|Qt::AlignVCenter</set>

<set>Qt::AlignRight|Qt::AlignTop</set>

like previously set in dlgtrackinfo.cpp

(same in dlgtrackinfomulti.ui I think)

ronso0 · 2024-11-05T11:25:15Z

src/library/dlgtrackinfo.ui

+              </property>
+              <property name="focusPolicy">
+                <enum>Qt::StrongFocus</enum>
+              </property>


I think this needs to be nested in a layout to restore the alignment (right | v-center)

(same in dlgtrackinfomulti.ui I think)

ronso0 · 2024-11-05T11:31:36Z

src/widget/wbasiclabel.h

+/// Subclass of QLabel that allows the Up and Down arrow keys
+/// to be used like the Tab/Backtab keys.


Suggested change

/// Subclass of QLabel that allows the Up and Down arrow keys

/// to be used like the Tab/Backtab keys.

/// Subclass of QLabel for DlgTrackInfo/~Multi and similar dialogs

/// that allows the Up and Down arrow keys to move keyboard focus

/// to the prev/next focusable widget.

Just to clarify that it's not supposed to be used in skins.

Or add that info to the line below

or prefix the class, eg. WDlgLabel?
(applies to all new sub-classes)

cr7pt0gr4ph7 added 30 commits June 22, 2024 20:55

DlgTrackInfo/LastPlayed: Add "Last played" field to track info dialog

f81e8d2

DlgTrackInfo/LastPlayed: Display placeholder for last played date

0eda4b9

DlgTrackInfo/Layout: Fix alignment of "Grouping" and "Comments" label

79b6d7f

This makes these two labels have the same alignment and pixel position (relative to their input control) as all the other labels in the dialog.

DlgTrackInfo/Cleanup: Fix: Make text of "Bitrate" field selectable

c00dfc9

All other similar fields in the dialog are selectable, so this seems to just have been an oversight.

DlgTrackInfo/Cleanup: Improve GroupBox names

9aa4ff0

DlgTrackInfo/Cleanup: Group related properties in the header

2e5a11b

DlgTrackInfo/Cleanup: Simplify updateTrackMetadataFields

9b9aa75

DlgTrackInfo/Cleanup: Associate read-only fields with track property …

f1f3089

…names

WStarRating: Remove Qt5 compatibility code

25eb63c

WStarRating: Add keyboard controls for modifying the star rating

4b309a8

WStarRating: Highlight the control when having keyboard focus

43ef74b

DlgTrackInfo/StarRating: Move UI setup code into the .ui file.

ae81d1f

DlgTrackInfo/StarRating: Integrate WStarRating into the tab order

5bef6d2

DlgTrackInfo/LeftRight: Conditionally enable/disable btnNext and btnPrev

f60d4c2

DlgTrackInfo/LeftRight: Add Alt+Left / Alt+Right as additional shortc…

ae797c2

…uts for navigating between tracks

DlgTrackInfo/LeftRight: Refocus the current widget when moving to a d…

643533e

…ifferent track

DlgTrackInfo/Sizing: Scroll to start of text fields by setting the cu…

dcd791b

…rsor position

DlgTrackInfo/Sizing: Add WMultiLineTextEdit to allow for correct resi…

891d021

…zing Also fix the sizePolicy of other widgets, such that the "Comments" field gets all available remaining vertical space.

WMultiLineTextEdit: Style like QLineEdit (including the focus frame)

3fba164

DlgTrackInfo/Sizing: Add WBasicPushButton to elide text

fe5c1f9

WBasicPushButton: Automatically generate tooltips when button is smal…

395697b

…ler than contents

DlgTrackInfo/UpDown: Change focus when pressing Up/Down on WBasicLine…

4085793

…Edit

DlgTrackInfo/UpDown: Change focus when pressing Up/Down on WMultiLine…

239717d

…TextField

DlgTrackInfo/UpDown: Change focus when pressing Up/Down on WStarRating

c6a016d

DlgTrackInfo/UpDown: Change focus when pressing Up/Down on WBasicTabW…

c83f4cb

…idget

DlgTrackInfo/UpDown: Change focus when pressing Up/Down on WBasicLabel

c0ff995

DlgTrackInfo/UpDown: Sort WBasicLabel controls by visual & tab order

a169351

This is needed to avoid breaking keyboard navigation in a few cases.

DlgTrackInfo/ColorPicker: Avoid double-triggering the color picker popup

5b2424d

The menu is already automatically opened by QPushButton::pressed, no need to do it twice. This caused problems when Enter is used to open the color picker popup.

WColorPicker: Fix oversight in idealColumnCount

d23e53d

WColorPicker: Allow keyboard navigation in the color picker popup

42f0b14

cr7pt0gr4ph7 added 3 commits June 22, 2024 20:57

DlgTrackInfo: Add tooltip for cover image

dbf2da0

Merge remote-tracking branch 'upstream/main' into ready-for-merge/tra…

8aa703f

…ck-info-dialog Resolved conflicts caused by mixxxdj#13632 (Move metatdata buttons below color picker).

DlgTrackInfo: Rename meatdata_buttons_layout to metadata_buttons_layout

a5ae64e

github-actions bot added library ui build labels Oct 31, 2024

cr7pt0gr4ph7 changed the title ~~Usability/Keyboard: Polish the track info dialog~~ Usability/Keyboard: Polish up the Track Info dialog Oct 31, 2024

WColorPicker: Fix code formatting issue in WColorGridButton constructor

5ad242b

cr7pt0gr4ph7 changed the title ~~Usability/Keyboard: Polish up the Track Info dialog~~ Track Info Dialog: Improve keyboard usability & resize behavior Oct 31, 2024

DlgTrackInfo/Cleanup: Associate "rating" and "color" property names w…

25c005f

…ith their corresponding edit controls

cr7pt0gr4ph7 force-pushed the ready-for-merge/track-info-dialog branch from 2d8fdf7 to 25c005f Compare October 31, 2024 19:30

DlgTrackInfoMulti/StarRating: Integrate WStarRating into the tab order

0f3fefd

cr7pt0gr4ph7 force-pushed the ready-for-merge/track-info-dialog branch from 033e06c to 0f3fefd Compare October 31, 2024 20:36

ronso0 requested changes Nov 1, 2024

View reviewed changes

cr7pt0gr4ph7 and others added 2 commits November 2, 2024 00:47

Use QKeyEvent::text() for handling Key_0..9

335c4cb

Co-authored-by: ronso0 <[email protected]>

Use 'const' and QStringLiteral where possible

7266f4d

Co-authored-by: ronso0 <[email protected]>

cr7pt0gr4ph7 commented Nov 2, 2024

View reviewed changes

cr7pt0gr4ph7 requested a review from ronso0 November 3, 2024 13:15

ronso0 requested changes Nov 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track Info Dialog: Improve keyboard usability & resize behavior #13818

Track Info Dialog: Improve keyboard usability & resize behavior #13818

cr7pt0gr4ph7 commented Oct 31, 2024 •

edited

Loading

ronso0 commented Oct 31, 2024

ronso0 commented Nov 1, 2024

cr7pt0gr4ph7 commented Nov 1, 2024 •

edited

Loading

ronso0 left a comment

ronso0 Nov 1, 2024

cr7pt0gr4ph7 Nov 2, 2024

ronso0 Nov 3, 2024

ronso0 Nov 1, 2024

cr7pt0gr4ph7 Nov 2, 2024

ronso0 Nov 3, 2024

cr7pt0gr4ph7 commented Nov 2, 2024

cr7pt0gr4ph7 Nov 2, 2024 •

edited

Loading

ronso0 Nov 3, 2024

cr7pt0gr4ph7 Nov 2, 2024 •

edited

Loading

cr7pt0gr4ph7 Nov 2, 2024 •

edited

Loading

cr7pt0gr4ph7 Nov 2, 2024

cr7pt0gr4ph7 Nov 2, 2024 •

edited

Loading

ronso0 commented Nov 3, 2024

ronso0 commented Nov 3, 2024

ronso0 commented Nov 3, 2024

ronso0 commented Nov 3, 2024

ronso0 commented Nov 3, 2024

ronso0 commented Nov 3, 2024

ronso0 commented Nov 5, 2024

ronso0 left a comment

ronso0 Nov 5, 2024

ronso0 Nov 5, 2024

ronso0 Nov 5, 2024

ronso0 Nov 5, 2024

ronso0 Nov 5, 2024

ronso0 Nov 5, 2024

	<set>Qt::AlignRight\|Qt::AlignVCenter</set>
	<set>Qt::AlignRight\|Qt::AlignTop</set>

		/// Subclass of QLabel that allows the Up and Down arrow keys
		/// to be used like the Tab/Backtab keys.

-/// Subclass of QLabel that allows the Up and Down arrow keys
-/// to be used like the Tab/Backtab keys.
+/// Subclass of QLabel for DlgTrackInfo/~Multi and similar dialogs
+/// that allows the Up and Down arrow keys to move keyboard focus
+/// to the prev/next focusable widget.

Track Info Dialog: Improve keyboard usability & resize behavior #13818

Are you sure you want to change the base?

Track Info Dialog: Improve keyboard usability & resize behavior #13818

Conversation

cr7pt0gr4ph7 commented Oct 31, 2024 • edited Loading

Summary

Add "Last Played" field (2 commits)

General Cleanup (6 commits)

Edit star rating via keyboard (5 commits)

Improve track list navigation (3 commits)

Resize behaviour (5 commits)

Keyboard navigation using Up/Down (6 commits)

Edit color picker via keyboard (3 commits)

Tooltip for cover art image (1 commit)

Merge & additional cleanup (2 commits)

Footnotes

ronso0 commented Oct 31, 2024

ronso0 commented Nov 1, 2024

cr7pt0gr4ph7 commented Nov 1, 2024 • edited Loading

ronso0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cr7pt0gr4ph7 commented Nov 2, 2024

cr7pt0gr4ph7 Nov 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cr7pt0gr4ph7 Nov 2, 2024 • edited Loading

Choose a reason for hiding this comment

cr7pt0gr4ph7 Nov 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cr7pt0gr4ph7 Nov 2, 2024 • edited Loading

Choose a reason for hiding this comment

ronso0 commented Nov 3, 2024

ronso0 commented Nov 3, 2024

ronso0 commented Nov 3, 2024

ronso0 commented Nov 3, 2024

ronso0 commented Nov 3, 2024

ronso0 commented Nov 3, 2024

ronso0 commented Nov 5, 2024

ronso0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cr7pt0gr4ph7 commented Oct 31, 2024 •

edited

Loading

Keyboard navigation using `Up`/`Down` (6 commits)

cr7pt0gr4ph7 commented Nov 1, 2024 •

edited

Loading

cr7pt0gr4ph7 Nov 2, 2024 •

edited

Loading

cr7pt0gr4ph7 Nov 2, 2024 •

edited

Loading

cr7pt0gr4ph7 Nov 2, 2024 •

edited

Loading

cr7pt0gr4ph7 Nov 2, 2024 •

edited

Loading