[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [amarok] /: QtGroupingProxy: implement buddy() to work-around design issues
From:       Matěj_Laitl <matej () laitl ! cz>
Date:       2012-12-31 16:17:07
Message-ID: 20121231161707.74F85A60C6 () git ! kde ! org
[Download RAW message or body]

Git commit 369508c70b7d9ced37afc0ddfdb661d72d17c3ab by Matěj Laitl.
Committed on 31/12/2012 at 16:25.
Pushed by laitl into branch 'master'.

QtGroupingProxy: implement buddy() to work-around design issues

This is effectively a work-around for a big design flaw in
QtGroupingProxy (which is that it "invents" its own items not present
in the original model). Technical description in the code comments.

^^^^ Bart, because of the above reason I'd prefer not to use
QtGroupingProxy in Amarok at all in long-term. Instead, we should do it
the other way around: original model would be hierarchical and the
proxy would flatten it into a table.

This fixes some bugs in the Saved Playlists and perhaps more:

BUGFIXES:
 * Fix editability and drop-ability of playlist folders.

CCBUG: 308703
CCMAIL: Bart Cerneels <bart.cerneels@kde.org>

M  +1    -0    ChangeLog
M  +0    -1    src/browsers/playlistbrowser/PlaylistBrowserCategory.cpp
M  +23   -0    src/browsers/playlistbrowser/QtGroupingProxy.cpp
M  +1    -0    src/browsers/playlistbrowser/QtGroupingProxy.h

http://commits.kde.org/amarok/369508c70b7d9ced37afc0ddfdb661d72d17c3ab

diff --git a/ChangeLog b/ChangeLog
index de3fc15..403e4bc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,6 +9,7 @@ VERSION 2.7
     * Removed alpha state and not really working spectrum analyzer applet.
 
   BUGFIXES:
+    * Fix editability and drop-ability of playlist folders.
     * Make track announcements via KDE Notifications instant. (BR 263952)
     * Fix dynamic playlist progress bar.
     * Fix Last played bias is ignored or doesn't work. (BR 311906)
diff --git a/src/browsers/playlistbrowser/PlaylistBrowserCategory.cpp \
b/src/browsers/playlistbrowser/PlaylistBrowserCategory.cpp index b2f449c..f6f06d6 \
                100644
--- a/src/browsers/playlistbrowser/PlaylistBrowserCategory.cpp
+++ b/src/browsers/playlistbrowser/PlaylistBrowserCategory.cpp
@@ -289,7 +289,6 @@ PlaylistBrowserCategory::createNewFolder()
     }
     QModelIndex idx = m_filterProxy->mapFromSource( \
m_byFolderProxy->createNewFolder( groupName ) );  m_playlistView->setCurrentIndex( \
                idx );
-    //edit will fail: https://bugreports.qt-project.org/browse/QTBUG-26838
     m_playlistView->edit( idx );
 }
 
diff --git a/src/browsers/playlistbrowser/QtGroupingProxy.cpp \
b/src/browsers/playlistbrowser/QtGroupingProxy.cpp index 55edbb4..24f611c 100644
--- a/src/browsers/playlistbrowser/QtGroupingProxy.cpp
+++ b/src/browsers/playlistbrowser/QtGroupingProxy.cpp
@@ -649,6 +649,29 @@ QtGroupingProxy::flags( const QModelIndex &idx ) const
     return originalItemFlags;
 }
 
+QModelIndex
+QtGroupingProxy::buddy( const QModelIndex &index ) const
+{
+    /* We need to override this method in case of groups. Otherwise, at least \
editing +     * of groups is prevented, following sequence occurs:
+     *
+     * #0  QtGroupingProxy::mapToSource (this=0x15ad8a0, index=...) at \
/home/strohel/projekty/amarok/src/browsers/playlistbrowser/QtGroupingProxy.cpp:492 +  \
* #1  0x00007ffff609d7b6 in QAbstractProxyModel::buddy (this=0x15ad8a0, index=...) at \
itemviews/qabstractproxymodel.cpp:306 +     * #2  0x00007ffff609ed25 in \
QSortFilterProxyModel::buddy (this=0x15ae730, index=...) at \
itemviews/qsortfilterproxymodel.cpp:2015 +     * #3  0x00007ffff6012a2c in \
QAbstractItemView::edit (this=0x15aec30, index=..., \
trigger=QAbstractItemView::AllEditTriggers, event=0x0) at \
itemviews/qabstractitemview.cpp:2569 +     * #4  0x00007ffff6f9aa9f in \
Amarok::PrettyTreeView::edit (this=0x15aec30, index=..., \
trigger=QAbstractItemView::AllEditTriggers, event=0x0) +     *     at \
/home/strohel/projekty/amarok/src/widgets/PrettyTreeView.cpp:58 +     * #5  \
0x00007ffff6007f1e in QAbstractItemView::edit (this=0x15aec30, index=...) at \
itemviews/qabstractitemview.cpp:1138 +     * #6  0x00007ffff6dc86e4 in \
PlaylistBrowserNS::PlaylistBrowserCategory::createNewFolder (this=0x159bf90) +     *  \
at /home/strohel/projekty/amarok/src/browsers/playlistbrowser/PlaylistBrowserCategory.cpp:298
 +     *
+     * but we return invalid index in mapToSource() for group index.
+     */
+    if( index.isValid() && isGroup( index ) )
+        return index;
+    return QAbstractProxyModel::buddy( index );
+}
+
 QVariant
 QtGroupingProxy::headerData( int section, Qt::Orientation orientation, int role ) \
const  {
diff --git a/src/browsers/playlistbrowser/QtGroupingProxy.h \
b/src/browsers/playlistbrowser/QtGroupingProxy.h index f7a5ed4..b3fddc8 100644
--- a/src/browsers/playlistbrowser/QtGroupingProxy.h
+++ b/src/browsers/playlistbrowser/QtGroupingProxy.h
@@ -48,6 +48,7 @@ class QtGroupingProxy : public QAbstractProxyModel
         virtual QModelIndex index( int row, int column = 0,
                                    const QModelIndex& parent = QModelIndex() ) \
const;  virtual Qt::ItemFlags flags( const QModelIndex &idx ) const;
+        virtual QModelIndex buddy( const QModelIndex &index ) const;
         virtual QModelIndex parent( const QModelIndex &idx ) const;
         virtual int rowCount( const QModelIndex &idx = QModelIndex() ) const;
         virtual int columnCount( const QModelIndex &idx ) const;


[prev in list] [next in list] [prev in thread] [next in thread]