QML: Add library table by Holzhaus · Pull Request #4594 · mixxxdj/mixxx

Holzhaus · 2021-12-30T23:11:10Z

Based on #4567 and #4654.

ywwg · 2022-02-03T16:45:24Z

similar to the waveform PR, I would like to keep QML moving quickly. If it's possible to get some experimental code merged in as a start, let's do it.

Holzhaus · 2022-02-07T21:11:17Z

Ok, I think this is ready to review.

I'm particulary interested in feedback regard the custom TableFromListModel class, which remaps roles from a listmodel into a tablemodel (so that we can use it as model for a QML TableView). The implementation is based on what Qt's TableModel QML type does.

The advantage of this is that the skin designer can decide what rows should be displayed, what order they should have, etc. and does not have to make assumptions about what column of the table contains what. Our playlist is displayed as a table, but semantically it's just a list of tracks, not a table.

Holzhaus · 2022-02-12T16:12:44Z

Rebased on #4673 to make stuff like SelectTrackKnob work.

Holzhaus · 2022-02-16T16:12:59Z

Rebased, ready to review again.

daschuer · 2022-02-17T08:14:35Z

+                                                                              \
+    void QmlTableFromListModelColumn::getterSetterName(                       \
+            const QJSValue& stringOrFunction) {                               \
+        if (!stringOrFunction.isString() && !stringOrFunction.isCallable()) { \


Due to the macro nature all this code is duplicated. It would be better to use helper functions for this.
This also helps to get around of some macro weirdness when debugging.

Maybe there is a chance to omit using the macro entirely. Currently it has already 7 parameter.
Maybe we can replace it wit 7 lines of c++ code forwarding to a template or something.

If you wish to go with the macro solution you may consider to use only the property name as parameter.
Like shown here:

mixxx/src/util/macros.h

Line 21 in d67b057

#define MIXXX_DECL_PROPERTY(TYPE, NAME, CAP_NAME) \

But I would prefer a c++ based solution even if it involves more writing initaly.

This macro-based solution works fine IMHO. If you want to replace the macros in a follow-up PR, feel free to do so.

uklotzde · 2022-02-17T12:08:35Z

@Holzhaus With a fresh profile the library panel remains empty. Warnings only affect other QML files.

Holzhaus · 2022-03-07T23:14:11Z

@uklotzde

With a fresh profile the library panel remains empty.

What do you mean by "it remains empty"? Is this a regression from the current QML library?

Holzhaus · 2022-03-10T14:18:34Z

@uklotzde

With a fresh profile the library panel remains empty.

What do you mean by "it remains empty"? Is this a regression from the current QML library?

Ah, this is unrelated to this PR and also happens on main (with QML). The reason for this is the weird way the model is populated. It requires calling TrackModel::select(), and apparently the legacy GUI code takes care of that....

A workaround for main could look like this:

diff --git a/res/qml/Library.qml b/res/qml/Library.qml
index 657cb58203..917e73147d 100644
--- a/res/qml/Library.qml
+++ b/res/qml/Library.qml
@@ -43,6 +43,7 @@ Item {
             }

             function loadSelectedTrackIntoNextAvailableDeck(play) {
+                model.select();
                 const url = model.get(currentIndex).fileUrl;
                 if (!url)
                     return ;
@@ -51,6 +52,7 @@ Item {
             }

             function loadSelectedTrack(group, play) {
+                model.select();
                 const url = model.get(currentIndex).fileUrl;
                 if (!url)
                     return ;
diff --git a/src/qml/qmllibrarytracklistmodel.h b/src/qml/qmllibrarytracklistmodel.h
index 490ff4508c..65b89cce46 100644
--- a/src/qml/qmllibrarytracklistmodel.h
+++ b/src/qml/qmllibrarytracklistmodel.h
@@ -1,4 +1,5 @@
 #pragma once
+#include "library/trackmodel.h"
 #include <QIdentityProxyModel>
 #include <QtQml>

@@ -25,6 +26,10 @@ class QmlLibraryTrackListModel : public QIdentityProxyModel {
     QmlLibraryTrackListModel(LibraryTableModel* pModel, QObject* pParent = nullptr);
     ~QmlLibraryTrackListModel() = default;

+    Q_INVOKABLE void select() {
+        dynamic_cast<TrackModel*>(sourceModel())->select();
+    }
+
     QVariant data(const QModelIndex& index, int role) const override;
     int columnCount(const QModelIndex& index = QModelIndex()) const override;
     QHash<int, QByteArray> roleNames() const override;

Now, if you double click a track in the table after dragging and dropping a new track onto a deck, the new track shows up. :-/

github-actions · 2022-06-09T00:06:43Z

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

Holzhaus · 2023-05-15T21:28:54Z

Merge?

Swiftb0y

I'm still torn on the amount of boilerplate required for this. I'll take a closer look asap. Thank you for reviving this again.

Swiftb0y

don't think I can find the time for much more right now.

Swiftb0y · 2023-05-16T14:02:06Z

+    Q_PROPERTY(int columnCount READ columnCount NOTIFY columnCountChanged FINAL)
+    Q_PROPERTY(QAbstractItemModel* sourceModel READ sourceModel WRITE
+                    setSourceModel NOTIFY sourceModelChanged REQUIRED FINAL)
+    Q_PROPERTY(QQmlListProperty<mixxx::qml::QmlTableFromListModelColumn> columns
+                    READ columns CONSTANT FINAL)


have you experimented with QProperty, could it reduce some of the boilerplate?

Swiftb0y · 2023-05-16T16:00:08Z

+
+class QmlTableFromListModel : public QAbstractTableModel, public QQmlParserStatus {


Can you add an explanation for why this class exists exactly?

acolombier · 2023-12-06T13:39:05Z

+            Keys.onEnterPressed: this.loadSelectedTrackIntoNextAvailableDeck(false)
+            Keys.onReturnPressed: this.loadSelectedTrackIntoNextAvailableDeck(false)
+            columnWidthProvider: function(column) {
+                switch (column) {


Is this related to the model defined in line 41? Could we make it so the custom default width is defined in model instead of here? Perhaps moving the model to be global property of library?

acolombier · 2023-12-06T13:42:31Z

+                Mixxx.TableFromListModelColumn {
+                    decoration: "color"
+                    edit: "fileUrl"
+                }


Could a Repeater be used here with the model defined at line 41?

acolombier · 2023-12-06T14:00:02Z

+        const QHash<int, QByteArray>& sourceRoleNames = m_pSourceModel->roleNames();
+        for (auto it = sourceRoleNames.cbegin(); it != sourceRoleNames.cend(); it++) {
+            int roleId = it.key();
+            m_sourceRoles.insert(QString::fromUtf8(it.value()), roleId);
+        }


Is the for-each loop format not possible here?

I don't think so, we want both key and value.

There is a new method asKeyValueRange in Qt 6.4 allowing this. Not sure if it is worth having behind preprocessor for when we upgrade to this Qt version?

Suggested change

const QHash<int, QByteArray>& sourceRoleNames = m_pSourceModel->roleNames();

for (auto it = sourceRoleNames.cbegin(); it != sourceRoleNames.cend(); it++) {

int roleId = it.key();

m_sourceRoles.insert(QString::fromUtf8(it.value()), roleId);

}

const QHash<int, QByteArray>& sourceRoleNames = m_pSourceModel->roleNames();

for (auto [roleId, value] : sourceRoleNames.asKeyValueRange()) {

m_sourceRoles.insert(QString::fromUtf8(value), roleId);

}

acolombier · 2023-12-07T09:12:41Z

+    return m_getters;
+}
+const QHash<int, QString> QmlTableFromListModelColumn::supportedRoleNames() {
+    QHash<int, QString> names;


Perhaps this could be a constant like kRoleNames?

acolombier · 2023-12-07T09:14:03Z

+    QJSValue setterAtRole(const QString& roleName);
+    const QHash<QString, QJSValue> getters() const;
+    static const QHash<int, QString> supportedRoleNames();
+  Q_SIGNALS:


perhaps using signals would be more consistent with the rest of the codebase

acolombier · 2025-02-05T18:27:15Z

Marking this as draft for now @Holzhaus - feel free to mark it ready again once we should give it another go!

Holzhaus added the library label Dec 30, 2021

github-actions Bot added the build label Dec 30, 2021

Holzhaus force-pushed the qml-library-table branch from 2a9d6a7 to 455e0fc Compare January 31, 2022 21:07

Holzhaus force-pushed the qml-library-table branch 2 times, most recently from c30b370 to 83a2d0c Compare February 7, 2022 21:02

Holzhaus changed the title ~~QML: Add library table [WIP]~~ QML: Add library table Feb 7, 2022

Holzhaus marked this pull request as ready for review February 7, 2022 21:11

Holzhaus marked this pull request as draft February 12, 2022 16:12

Holzhaus force-pushed the qml-library-table branch 2 times, most recently from a465bb4 to a0a0c6e Compare February 16, 2022 16:04

Holzhaus marked this pull request as ready for review February 16, 2022 16:12

Holzhaus requested a review from uklotzde February 16, 2022 16:13

daschuer requested changes Feb 17, 2022

View reviewed changes

github-actions Bot added the stale Stale issues that haven't been updated for a long time. label Jun 9, 2022

Holzhaus force-pushed the qml-library-table branch from 7251d5d to 17fb124 Compare January 3, 2023 22:32

Holzhaus force-pushed the qml-library-table branch 2 times, most recently from fa54d6e to 7f250a3 Compare April 7, 2023 21:00

Holzhaus added 5 commits April 7, 2023 23:14

feat(qml): Add TableFromListModel QML type to map list roles to table

fbb0871

feat(qml): Use TableView for Library

d3f5259

chore(qml): Add support for more roles to QmlLibraryTrackListModel

3e93274

feat(qml): Add more columns to library table

a75289f

feat(qml): Add table headers to library

b407ed4

chore(qml): Forward signals in QmlTableFromListModel

2280857

Holzhaus force-pushed the qml-library-table branch from 7f250a3 to 2280857 Compare April 7, 2023 21:14

Merge branch 'main' of github.com:mixxxdj/mixxx into qml-library-table

ee0a18b

Swiftb0y reviewed May 15, 2023

View reviewed changes

Swiftb0y reviewed May 16, 2023

View reviewed changes

ronso0 mentioned this pull request Aug 17, 2023

Additional search fields #11824

Open

github-actions Bot removed the stale Stale issues that haven't been updated for a long time. label Aug 31, 2023

acolombier reviewed Dec 7, 2023

View reviewed changes

acolombier marked this pull request as draft February 5, 2025 18:27

acolombier mentioned this pull request Mar 24, 2025

feat: add Library on QML #14514

Merged

acolombier added the stale Stale issues that haven't been updated for a long time. label Mar 24, 2025

acolombier self-assigned this Jun 18, 2025

github-actions Bot removed the stale Stale issues that haven't been updated for a long time. label Sep 11, 2025


		class QmlTableFromListModel : public QAbstractTableModel, public QQmlParserStatus {

Uh oh!

Conversation

Holzhaus commented Dec 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ywwg commented Feb 3, 2022

Uh oh!

Holzhaus commented Feb 7, 2022

Uh oh!

Holzhaus commented Feb 12, 2022

Uh oh!

Holzhaus commented Feb 16, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

uklotzde commented Feb 17, 2022

Uh oh!

Holzhaus commented Mar 7, 2022

Uh oh!

Holzhaus commented Mar 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 9, 2022

Uh oh!

Holzhaus commented May 15, 2023

Uh oh!

Swiftb0y left a comment

Choose a reason for hiding this comment

Uh oh!

Swiftb0y left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acolombier commented Feb 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Holzhaus commented Dec 30, 2021 •

edited

Loading

Holzhaus commented Mar 10, 2022 •

edited

Loading