renatoo added a comment.
In https://phabricator.kde.org/D8243#173442, @broulik wrote:
> Can we expose the group type enum in the model in addition to the name, so
I can do some filtering in e.g. task manager?
This will be exposed by this mr:
broulik added a comment.
Can we expose the group type enum in the model in addition to the name, so I
can do some filtering in e.g. task manager?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg, ngraham
Cc: broulik,
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:7a6eff3fce41: Implement support for categories on
KfilesPlacesView (authored by Renato Araujo Oliveira Filho
renato.ara...@kdab.com, committed by ngraham).
REPOSITORY
R241 KIO
CHANGES SINCE LAST
ngraham accepted this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg, ngraham
Cc: markg, ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
dfaure accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: markg, ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
ervin added a comment.
Great work BTW, this looks really nice now.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: markg, ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
ervin accepted this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: markg, ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
renatoo updated this revision to Diff 21582.
renatoo added a comment.
Updated visuals to match dolphin
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8243?vs=21226=21582
REVISION DETAIL
https://phabricator.kde.org/D8243
AFFECTED FILES
renatoo marked 3 inline comments as done.
renatoo added inline comments.
INLINE COMMENTS
> ervin wrote in kfileplacesmodel.cpp:476
> Either reorder the enum or change for a different (more explicit not relying
> on enum storage) comparison operator to have the "right" order. By right
> order I
renatoo added inline comments.
INLINE COMMENTS
> ervin wrote in kfileplacesmodel.cpp:476
> Either reorder the enum or change for a different (more explicit not relying
> on enum storage) comparison operator to have the "right" order. By right
> order I assume the goal was to align with Dolphin
ervin added a comment.
Looks fine code wise now, just a couple more tweaks to make those sections
look closer to what Dolphin got.
INLINE COMMENTS
> kfileplacesmodel.cpp:476
> +[](KFilePlacesItem *itemA, KFilePlacesItem *itemB) {
> + return (itemA->groupType() <
renatoo added a comment.
Guys, is that ready? Do you need any other change/fix?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: markg, ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
renatoo marked 2 inline comments as done.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: markg, ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
renatoo updated this revision to Diff 21226.
renatoo added a comment.
Fixed typo
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8243?vs=20951=21226
REVISION DETAIL
https://phabricator.kde.org/D8243
AFFECTED FILES
autotests/kfileplacesmodeltest.cpp
dfaure added inline comments.
INLINE COMMENTS
> kfileplacesview.cpp:1146
> +
> +const int totlaItemsHeight = (fm.height() / 2) * rowCount;
> +const int totoalSectionsHeight = delegate->sectionHeaderHeight() *
> sectionsCount();
typo (total)
> kfileplacesview.cpp:1147
> +const int
ngraham added a comment.
@dfaure and @ervin, is this acceptable now?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: markg, ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
renatoo marked 3 inline comments as done.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: markg, ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
franckarrecot added a dependent revision: D8367: Hidding place groups
implementation in KFilePlacesModel.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: markg, ngraham, mlaurent, ervin, anthonyfieroni, cfeck,
ngraham added a comment.
@renatoo, there are still a few Not Done comments. Any chance you can resolve
them so we can push this forward?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: markg, ngraham, mlaurent,
ngraham added a comment.
@markg
Ultimately I think everyone is in agreement in wanting the "single scrollbar"
approach for items in whatever you call the default list on Dolphin's left
side. Splitting everything into distinct panels and embedding all the panels in
a single scrollview
markg added a comment.
In https://phabricator.kde.org/D8243#157346, @renatoo wrote:
> In https://phabricator.kde.org/D8243#157336, @markg wrote:
>
> > We have such a nice modular design with panels yet we keep cramming
everything in the places panel... Why?
> > It's not just here,
renatoo added a comment.
In https://phabricator.kde.org/D8243#157336, @markg wrote:
> We have such a nice modular design with panels yet we keep cramming
everything in the places panel... Why?
> It's not just here, I've seen patches like this for Dolphin as well.
>
> Imho, the
markg added a comment.
We have such a nice modular design with panels yet we keep cramming
everything in the places panel... Why?
It's not just here, I've seen patches like this for Dolphin as well.
Imho, the places panel should only contain the top places - favorites if you
will -
renatoo marked 2 inline comments as done.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
ngraham added a comment.
@renatoo, can you update any remaining Not Done inline comments that are done
now (including the discussion ones)?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: ngraham, mlaurent, ervin,
renatoo updated this revision to Diff 20951.
renatoo added a comment.
Fixed places item size calculation
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8243?vs=20939=20951
REVISION DETAIL
https://phabricator.kde.org/D8243
AFFECTED FILES
dfaure added a comment.
+1, looks good to me, implementation wise. I'll let someone else decide on
the actual feature (e.g. retesting for bugs, like Kévin did).
I suspect that the use of QApplication::font/fontMetrics breaks when setting
a font on that widget, but well, who would do
renatoo updated this revision to Diff 20939.
renatoo marked 11 inline comments as done.
renatoo edited the summary of this revision.
renatoo added a comment.
Fixed typo and code style
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8243?vs=20675=20939
REVISION
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kfileplacesmodeltest.cpp:323
> +// Trying move the remote at the end of the list, should move it to the
> end of places section instead
> +// too
ngraham added a reviewer: VDG.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin, #vdg
Cc: ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
renatoo added a dependent revision: D8332: Added baloo urls into places model.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin
Cc: ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
renatoo added a dependent revision: D8348: Add a section for removable devices.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin
Cc: ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
ngraham added a comment.
FWIW, I don't have a lot to offer on the code itself, but I strongly support
the change itself. It's always bothered me that Open/Save dialogs were lacking
these very useful sections
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To:
renatoo edited the summary of this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin
Cc: ngraham, mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
ngraham added a comment.
Screenshots, my man! Screenshots! Add them to the Summary section so people
can see what they're getting if this gets merged. :)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin
Cc: ngraham, mlaurent,
renatoo updated this revision to Diff 20675.
renatoo added a comment.
Updated unit test
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8243?vs=20674=20675
BRANCH
model-hide-row
REVISION DETAIL
https://phabricator.kde.org/D8243
AFFECTED FILES
renatoo updated this revision to Diff 20674.
renatoo marked 8 inline comments as done.
renatoo edited the summary of this revision.
renatoo added a comment.
updated comit summary
renamed m_dragModeCount to m_dragStart
refactory 'KFilePlacesItem::data' function
REPOSITORY
R241 KIO
ervin added a comment.
A couple more changes needed.
INLINE COMMENTS
> kfileplacesitem.cpp:153-158
> +if (role == KFilePlacesModel::GroupRole) {
> +return QVariant(m_groupName);
> +}
>
> +QVariant returnData;
> if (role != KFilePlacesModel::HiddenRole && role !=
ervin requested changes to this revision.
ervin added a comment.
This revision now requires changes to proceed.
You probably missed my comment on the previous iteration:
"Note that the summary part of the commit is now wrong (it still refers to
KCategorizedView).
Also,
anthonyfieroni added inline comments.
INLINE COMMENTS
> renatoo wrote in kfileplacesview.cpp:169
> Humm I did not know about this opt.index property. Did some debug here and
> it is always -1 .
>
> Sorry but I am not sure why should I care about opt.index? Checking the
> "index" arg is not
renatoo added inline comments.
INLINE COMMENTS
> anthonyfieroni wrote in kfileplacesview.cpp:169
> Ok i understaind the workaround, but did you know what is the value of
> opt.index.row() ?
Humm I did not know about this opt.index property. Did some debug here and it
is always -1 .
Sorry
anthonyfieroni added inline comments.
INLINE COMMENTS
> renatoo wrote in kfileplacesview.cpp:169
> m_dragModeCount is used as workaround to draw items on floating window during
> the drag.
>
> When drag starts it save the number of selected items and until we draw all
> selected items once we
renatoo marked an inline comment as done.
renatoo added inline comments.
INLINE COMMENTS
> anthonyfieroni wrote in kfileplacesview.cpp:169
> Why you are sure that m_dragModeCount is 0 when indexIsSectionHeader? Why not
> when index.row() == 0 we can paint section header?
m_dragModeCount is
anthonyfieroni added inline comments.
INLINE COMMENTS
> anthonyfieroni wrote in kfileplacesview.cpp:1150
> This calculation makes no sense to me. We have ((x - (y/2) * rowCount) /
> rowCount so rowCount can be removed no?
Now i see i'm in wrong :)
REPOSITORY
R241 KIO
REVISION DETAIL
anthonyfieroni added inline comments.
INLINE COMMENTS
> kfileplacesview.cpp:169
> +// if is drawing the floating element used by drag/drop, does not
> draw the header
> +if (m_dragModeCount == 0) {
> +drawSectionHeader(painter, opt, index);
Why you are sure that
renatoo marked 2 inline comments as done.
renatoo added inline comments.
INLINE COMMENTS
> renatoo wrote in kfileplacesview.cpp:424
> I need a way query the header size height from "pointIsHeaderArea" to know if
> the user clicked on header or in the real item. Since I do not have access
>
renatoo updated this revision to Diff 20656.
renatoo added a comment.
Use QApplication::font() to determine the section header font size
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8243?vs=20650=20656
BRANCH
model-hide-row
REVISION DETAIL
renatoo added inline comments.
INLINE COMMENTS
> anthonyfieroni wrote in kfileplacesview.cpp:424
> So you mean sizeHint have a right height ? This height store looks like a
> workaround.
I need a way query the header size height from "pointIsHeaderArea" to know if
the user clicked on header
anthonyfieroni added inline comments.
INLINE COMMENTS
> kfileplacesview.cpp:424
> +if (m_sectionHeaderHeight == -1) {
> +m_sectionHeaderHeight = option.fontMetrics.height();
> +}
So you mean sizeHint have a right height ? This height store looks like a
workaround.
REPOSITORY
renatoo marked an inline comment as done.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin
Cc: mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
renatoo updated this revision to Diff 20650.
renatoo marked an inline comment as done.
renatoo added a comment.
Optimize sectionsCount to not use a QSet
Use static_cast when possible
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8243?vs=20646=20650
BRANCH
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kfileplacesview.cpp:997
> +{
> +KFilePlacesViewDelegate *delegate = dynamic_cast *>(itemDelegate());
> +
If you are sure that the delegate is a
renatoo updated this revision to Diff 20646.
renatoo added a comment.
Fixed code style
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8243?vs=20645=20646
BRANCH
model-hide-row
REVISION DETAIL
https://phabricator.kde.org/D8243
AFFECTED FILES
renatoo marked an inline comment as done.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks, dfaure, ervin
Cc: mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
renatoo added a comment.
All problems fixed.
INLINE COMMENTS
> anthonyfieroni wrote in kfileplacesview.cpp:1065-1066
> So if second line is correct why not remove first one?
Sorry wrong operator it should be "+="
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
renatoo updated this revision to Diff 20645.
renatoo marked 9 inline comments as done.
renatoo added a comment.
Fix code syntax
Disable drag from item header
Does not show item header on floating preview during the drag
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
mlaurent added inline comments.
INLINE COMMENTS
> kfileplacesview.cpp:113
> +QColor baseColor(const QStyleOption ) const;
> +QColor mixedColor(const QColor& c1, const QColor& c2, int c1Percent)
> const;
> };
coding style const QColor , const QColor
REPOSITORY
R241 KIO
REVISION
ervin added a comment.
One more thing, the way the sections are currently done, if I drag the first
item of a section the visual feedback I get also includes the section itself
giving the feeling that the section itself is being moved. Also I can drag from
the section title itself which is
ervin requested changes to this revision.
ervin added a comment.
This revision now requires changes to proceed.
Note that the summary part of the commit is now wrong (it still refers to
KCategorizedView).
Also, KFilePlacesModelTest needs to be adjusted to take into account the new
anthonyfieroni added inline comments.
INLINE COMMENTS
> kfileplacesview.cpp:104
> +
> +int m_sectionRole;
> +
const
> kfileplacesview.cpp:1065-1066
> const int maxWidth = q->viewport()->width() - textWidth - 4 * margin - 1;
> -const int maxHeight = ((q->height() - (fm.height() /
renatoo updated this revision to Diff 20613.
renatoo added a comment.
Does not use KCategorizedView
Instead of use KCategorizedView, draw the section over the first item area
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8243?vs=20587=20613
BRANCH
anthonyfieroni added a reviewer: dfaure.
anthonyfieroni added inline comments.
INLINE COMMENTS
> kfileplacesview.h:37
> */
> -class KIOFILEWIDGETS_EXPORT KFilePlacesView : public QListView
> +class KIOFILEWIDGETS_EXPORT KFilePlacesView : public KCategorizedView
> {
You cannot do that, you
cfeck added a reviewer: Frameworks.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8243
To: renatoo, #frameworks
Cc: cfeck, #frameworks
renatoo created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
REVISION SUMMARY
Make use of KCategorizedView on file dialogs to classify the entries in
groups,
most similar as possible with dolphin
TEST PLAN
From
64 matches
Mail list logo
