This revision was automatically updated to reflect the committed changes.
Closed by commit R241:b8bd61c3650c: User can now hide an entire places group
from KFilePlacesView (authored by franckarrecot).
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8450?vs=23630&id
franckarrecot updated this revision to Diff 23630.
franckarrecot added a comment.
corrected and rebased
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8450?vs=23410&id=23630
REVISION DETAIL
https://phabricator.kde.org/D8450
AFFECTED FILES
autotests/kfilep
renatoo added a dependent revision: D9241: Emit 'groupHiddenChanged' signal..
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8450
To: franckarrecot, ngraham, renatoo, ervin, mwolff, mlaurent
Cc: mwolff, #frameworks
renatoo accepted this revision.
renatoo added a comment.
Looks good and work as expected
INLINE COMMENTS
> kfileplacesview.cpp:766
> +}
> if (!clickOverHeader && index.isValid()) {
> if (!placesModel->isDevice(index)) {
I think that you can replace
if (!clickOverHeader &&
mwolff accepted this revision.
mwolff added a comment.
This revision is now accepted and ready to land.
one minor nit, otherwise lgtm
INLINE COMMENTS
> kfileplacesmodel.cpp:431
> +QModelIndexList indexes;
> +for (int row = 0; row < rowCount(); ++row) {
> +const QModelIndex cur
mlaurent accepted this revision.
mlaurent added inline comments.
INLINE COMMENTS
> ervin wrote in kfileplacesview.cpp:863-866
> This duplicates code from the next if branch below... I wonder if before or
> after that commit we shouldn't try to refactor that and remove some of the
> code duplica
franckarrecot updated this revision to Diff 23410.
franckarrecot added a comment.
rebase to master
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8450?vs=23025&id=23410
REVISION DETAIL
https://phabricator.kde.org/D8450
AFFECTED FILES
autotests/kfileplaces
ervin accepted this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8450
To: franckarrecot, ngraham, renatoo, ervin, mwolff, mlaurent
Cc: mwolff, #frameworks
franckarrecot updated this revision to Diff 23025.
franckarrecot marked an inline comment as done.
franckarrecot added a comment.
update, have to wait to be pushed, since it contains i18n translation, next
week:
in the first two weeks after the first saturday of the month
REPOSITORY
R241
franckarrecot marked an inline comment as done.
franckarrecot added inline comments.
INLINE COMMENTS
> ervin wrote in kfileplacesview.cpp:863-866
> Cool, I'll wait for that extra review to appear before accepting that one. So
> that we don't forget it. :-)
Was that the thing you had in mind ?
franckarrecot added a dependent revision: D9015: Refactoring the
hidding/showing animation use within KFilePlacesView.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8450
To: franckarrecot, ngraham, renatoo, ervin, mwolff, mlaurent
Cc: mwolff, #frameworks
franckarrecot updated this revision to Diff 22999.
franckarrecot added a comment.
now related to the refactoring commit coming after
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8450?vs=22902&id=22999
REVISION DETAIL
https://phabricator.kde.org/D8450
AFFE
franckarrecot edited the summary of this revision.
franckarrecot added dependencies: D8948: Created an auxiliary function
'KFilePlacesModel::movePlace', D8947: Expose KFilePlacesModel 'iconName' role,
D8946: Avoid unnecessary 'dataChanged' signal, D8945: Return a valid bookmark
object for any e
ervin added inline comments.
INLINE COMMENTS
> franckarrecot wrote in kfileplacesview.cpp:863-866
> Oki I add that to my list :-)
Cool, I'll wait for that extra review to appear before accepting that one. So
that we don't forget it. :-)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabrica
franckarrecot added inline comments.
INLINE COMMENTS
> ervin wrote in kfileplacesview.cpp:863-866
> I still stand by that comment, I'd welcome a patch on top of that one to
> reduce that duplication please.
Oki I add that to my list :-)
> ervin wrote in kfileplacesview.cpp:298
> That smells st
franckarrecot marked 3 inline comments as done.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8450
To: franckarrecot, ngraham, renatoo, ervin, mwolff, mlaurent
Cc: mwolff, #frameworks
franckarrecot updated this revision to Diff 22902.
franckarrecot marked an inline comment as done.
franckarrecot added a comment.
rebased over in review renato 's patches
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8450?vs=22734&id=22902
REVISION DETAIL
h
ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> ervin wrote in kfileplacesview.cpp:863-866
> This duplicates code from the next if branch below... I wonder if before or
> after that commit we shouldn't try to
franckarrecot updated this revision to Diff 22734.
franckarrecot added a comment.
update
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8450?vs=21644&id=22734
REVISION DETAIL
https://phabricator.kde.org/D8450
AFFECTED FILES
autotests/kfileplacesmodeltest.
franckarrecot commandeered this revision.
franckarrecot edited reviewers, added: mlaurent; removed: franckarrecot.
REVISION DETAIL
https://phabricator.kde.org/D8450
To: franckarrecot, ngraham, renatoo, ervin, mwolff, mlaurent
Cc: mwolff, #frameworks
franckarrecot added inline comments.
INLINE COMMENTS
> mwolff wrote in kfileplacesview.cpp:298
> the reserve + loop should be the same as doing
>
> m_disappearingItems += indexesGroup;
I'm filling a list of persistant indexes with regular indexes that is why I use
the loop, if you have any o
mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.
please use arc, or add more context to your patches in the future
INLINE COMMENTS
> kfileplacesview.cpp:298
> +
> +m_disappearingItems.reserve(m_disappearingItems.count() +
>
ngraham added a dependent revision: D8619: Refactor and remove duplicate code
in kfileplacesview.
REVISION DETAIL
https://phabricator.kde.org/D8450
To: mlaurent, ngraham, renatoo, franckarrecot, ervin
Cc: #frameworks
ngraham edited the summary of this revision.
ngraham added a dependency: D8367: Hidding place groups implementation in
KFilePlacesModel.
REVISION DETAIL
https://phabricator.kde.org/D8450
To: mlaurent, ngraham, renatoo, franckarrecot, ervin
Cc: #frameworks
ervin accepted this revision.
This revision is now accepted and ready to land.
REVISION DETAIL
https://phabricator.kde.org/D8450
To: mlaurent, ngraham, renatoo, franckarrecot, ervin
Cc: #frameworks
mlaurent updated this revision to Diff 21644.
mlaurent added a comment.
Autotest GroupIndexes
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8450?vs=21612&id=21644
REVISION DETAIL
https://phabricator.kde.org/D8450
AFFECTED FILES
autotests/kfileplacesmodeltest.cpp
src/filewidg
ervin requested changes to this revision.
ervin added a comment.
This revision now requires changes to proceed.
At least the unit test would be welcome, I let you decide on the other
comment.
INLINE COMMENTS
> kfileplacesmodel.cpp:329
>
> +QModelIndexList KFilePlacesModel::groupIndexes(con
mlaurent updated this revision to Diff 21612.
mlaurent added a comment.
Minor optimization.
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8450?vs=21291&id=21612
REVISION DETAIL
https://phabricator.kde.org/D8450
AFFECTED FILES
src/filewidgets/kfileplacesmodel.cpp
src/filewidg
mlaurent edited the test plan for this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8450
To: mlaurent, ngraham, renatoo, franckarrecot
Cc: #frameworks
mlaurent added a reviewer: ervin.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8450
To: mlaurent, ngraham, renatoo, franckarrecot, ervin
Cc: #frameworks
mlaurent edited the test plan for this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8450
To: mlaurent, ngraham, renatoo, franckarrecot
Cc: #frameworks
mlaurent commandeered this revision.
mlaurent edited reviewers, added: franckarrecot; removed: mlaurent.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8450
To: mlaurent, ngraham, renatoo, franckarrecot
Cc: #frameworks
franckarrecot added a comment.
I will add pics soonish
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8450
To: franckarrecot, ngraham, renatoo, mlaurent
Cc: #frameworks
franckarrecot updated this revision to Diff 21291.
franckarrecot added a comment.
updated according to comments
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8450?vs=21244&id=21291
REVISION DETAIL
https://phabricator.kde.org/D8450
AFFECTED FILES
src/file
franckarrecot retitled this revision from "User can now hide an entire places
group" to "User can now hide an entire places group from KFilePlacesView".
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8450
To: franckarrecot, ngraham, renatoo, mlaurent
Cc: #frameworks
35 matches
Mail list logo
