[prev in list] [next in list] [prev in thread] [next in thread]
List: kfm-devel
Subject: Re: Review Request 129533: Dolphin: expand all one level, expand all, and collapse all in details v
From: Emmanuel Pescosta <emmanuelpescosta099 () gmail ! com>
Date: 2017-01-14 11:21:11
Message-ID: 20170114112111.32006.79201 () mimi ! kde ! org
[Download RAW message or body]
--===============2381394263597058693==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
> On Jan. 4, 2017, 9:33 p.m., Andreas Kainz wrote:
> > is it already shiped? Personally I'm very happy that dolphin has not that much \
> > entriey, and you can show/hide them. when you have now Expand, Expand Full, \
> > Collapse All as a single entry at right click and in view the UI get a lot of \
> > additional stuff most people didn't use.
> > Menubar -> View
> > wan't it be better to have a expand group with subgroups Expand, Expand Full, \
> > Collapse All? why is it called Expand Full and Collapse All why not Expand All, \
> > Collapse All
> > Context menu (right click)
> > this is only available in the detail view isn't it. is it in a service cause than \
> > the user can add/remove the elements in Dolphin -> Preferences -> Services. I \
> > also would prefer to have here an expand main group with subgroups for Expand, \
> > Expand All, Collapse All, like we have it for open with and activities
> > I will make the breeze icons for the patch
>
> Don Nguyen wrote:
> In this context Full is used to describe how to expand the items, eg. completely, \
> and All is used to describe what to expand, eg. everything. The three functions \
> were a generalization to minimize the amount of menu items and text for the \
> following functions: Expand Selection Once, Expand All Once, Expand Selection \
> Full, Expand All Full, Collapse All.
> These functionalities are not a service - they are simply convenience functions.
>
> As the patch has not been shipped, we can still easily make any adjustments to how \
> these functions are exposed.
> If we go with a submenu route, we could do something like have a "Expand/Collapse" \
> submenu containing: "Expand Selected, Expand All, Expand Selected Completely, \
> Expand All Completely, Collapse All", and it would be a lot clearer what these \
> functions do. This design would add only one "Expand/Collapse" item to the View \
> menu and context menu. It would not even appear in the context menu, and would be \
> disabled in the View menu, when not in details view.
> is it already shiped?
I didn't have enough time yet to properly review this rather huge patch.
> I will make the breeze icons for the patch
Nice, thanks!
- Emmanuel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129533/#review101799
-----------------------------------------------------------
On Dec. 7, 2016, 10:53 p.m., Don Nguyen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129533/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2016, 10:53 p.m.)
>
>
> Review request for Dolphin and KDE Usability.
>
>
> Bugs: 196772
> https://bugs.kde.org/show_bug.cgi?id=196772
>
>
> Repository: dolphin
>
>
> Description
> -------
>
> Implemented functions: Expand, Expand Full, and Collapse All. They are available \
> under the View and Control menus as well as custom toolbar buttons as Thomas \
> explained and are disabled when not in details view mode. They are also available \
> via the context menu only in details view mode. See screenshots. Expand - If \
> there is a selection, expand the selection once. Otherwise expand everything once. \
> Expand Full - If there is a selection, expand the selected items completely. \
> Otherwise expand everything completely. Collapse All - Collapse everything.
>
>
> Diffs
> -----
>
> src/dolphincontextmenu.h f67300d
> src/dolphincontextmenu.cpp ed3f643
> src/dolphinmainwindow.cpp e5103fd
> src/dolphinui.rc f197af4
> src/kitemviews/kfileitemmodel.h 8a0df72
> src/kitemviews/kfileitemmodel.cpp 05f85a6
> src/tests/kfileitemmodeltest.cpp 6c45e11
> src/views/dolphinview.h 0b0d819
> src/views/dolphinview.cpp 4105628
> src/views/dolphinviewactionhandler.h eb375b5
> src/views/dolphinviewactionhandler.cpp 858f929
>
> Diff: https://git.reviewboard.kde.org/r/129533/diff/
>
>
> Testing
> -------
>
> make test passes
> manual testing including:
> expand all on an Android SDK folder which yielded in 3180 folders, and 30993 files
> collapse all on above situation
> expand all on a directory that contains a symbolic link to a directory containing \
> itself - operation will stop with error when expansion reaches a path that \
> KCoreDirLister doesn't accept(see looptest screenshot) changing directories in the \
> middle of an expand all operation changing directories in the middle of a restore \
> expanded directories operation canceling in the middle of an expand all operation
> canceling in the middle of a restore expanded directories operation
> canceling in the middle of an expand all operation or restore expand directory \
> operation, and then expanding an item manually with mouse click collapse all in the \
> middle of an expand all operation collapse all in the middle of a restore expanded \
> directories operation switched view modes and verified items are enabled only in \
> details view mode
>
> File Attachments
> ----------------
>
> looptest
> https://git.reviewboard.kde.org/media/uploaded/files/2016/11/25/f02b7e85-d0b3-4153-ae0d-e86a4ddfc916__looptest.png
> context-menu-disabled
> https://git.reviewboard.kde.org/media/uploaded/files/2016/12/04/1fcc4412-e5f2-4c31-92bf-b828eea938f7__contextdisabled.png
> context-menu-with-selection
> https://git.reviewboard.kde.org/media/uploaded/files/2016/12/04/8a4709e5-d674-4f74-8cbc-a71841db7c36__selectioncontextenabled.png
> view-menu-enabled
> https://git.reviewboard.kde.org/media/uploaded/files/2016/12/04/1ddaa88d-e0aa-441a-a840-5e2cdc06fbf2__viewmenuenabled.png
>
>
> Thanks,
>
> Don Nguyen
>
>
--===============2381394263597058693==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/129533/">https://git.reviewboard.kde.org/r/129533/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On January 4th, 2017, 9:33 p.m. CET, <b>Andreas \
Kainz</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">is it already shiped? Personally I'm very happy that \
dolphin has not that much entriey, and you can show/hide them. when you have now \
Expand, Expand Full, Collapse All as a single entry at right click and in view the UI \
get a lot of additional stuff most people didn't use.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Menubar -> View wan't it be better to have a expand group with subgroups \
Expand, Expand Full, Collapse All? why is it called Expand Full and Collapse All why \
not Expand All, Collapse All</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Context menu (right click) this is only \
available in the detail view isn't it. is it in a service cause than the user can \
add/remove the elements in Dolphin -> Preferences -> Services. I also would \
prefer to have here an expand main group with subgroups for Expand, Expand All, \
Collapse All, like we have it for open with and activities</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I \
will make the breeze icons for the patch</p></pre> </blockquote>
<p>On January 9th, 2017, 9:59 p.m. CET, <b>Don Nguyen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">In this context Full is \
used to describe how to expand the items, eg. completely, and All is used to describe \
what to expand, eg. everything. The three functions were a generalization to minimize \
the amount of menu items and text for the following functions: Expand Selection \
Once, Expand All Once, Expand Selection Full, Expand All Full, Collapse All.
These functionalities are not a service - they are simply convenience functions.
As the patch has not been shipped, we can still easily make any adjustments to how \
these functions are exposed.
If we go with a submenu route, we could do something like have a \
"Expand/Collapse" submenu containing: "Expand Selected, Expand All, \
Expand Selected Completely, Expand All Completely, Collapse All", and it would \
be a lot clearer what these functions do. This design would add only one \
"Expand/Collapse" item to the View menu and context menu. It would not \
even appear in the context menu, and would be disabled in the View menu, when not in \
details view.</pre> </blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> is it already \
shiped?
I didn't have enough time yet to properly review this rather huge patch.
> I will make the breeze icons for the patch
Nice, thanks!</pre>
<br />
<p>- Emmanuel</p>
<br />
<p>On December 7th, 2016, 10:53 p.m. CET, Don Nguyen wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Dolphin and KDE Usability.</div>
<div>By Don Nguyen.</div>
<p style="color: grey;"><i>Updated Dec. 7, 2016, 10:53 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=196772">196772</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
dolphin
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Implemented functions: Expand, Expand Full, and Collapse All. They are \
available under the View and Control menus as well as custom toolbar buttons as \
Thomas explained and are disabled when not in details view mode. They are also \
available via the context menu only in details view mode. See screenshots. Expand \
- If there is a selection, expand the selection once. Otherwise expand everything \
once. Expand Full - If there is a selection, expand the selected items completely. \
Otherwise expand everything completely. Collapse All - Collapse everything.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">make test passes manual testing including:
expand all on an Android SDK folder which yielded in 3180 folders, and 30993 files
collapse all on above situation
expand all on a directory that contains a symbolic link to a directory containing \
itself - operation will stop with error when expansion reaches a path that \
KCoreDirLister doesn't accept(see looptest screenshot) changing directories in \
the middle of an expand all operation changing directories in the middle of a restore \
expanded directories operation canceling in the middle of an expand all operation
canceling in the middle of a restore expanded directories operation
canceling in the middle of an expand all operation or restore expand directory \
operation, and then expanding an item manually with mouse click collapse all in the \
middle of an expand all operation collapse all in the middle of a restore expanded \
directories operation switched view modes and verified items are enabled only in \
details view mode</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/dolphincontextmenu.h <span style="color: grey">(f67300d)</span></li>
<li>src/dolphincontextmenu.cpp <span style="color: grey">(ed3f643)</span></li>
<li>src/dolphinmainwindow.cpp <span style="color: grey">(e5103fd)</span></li>
<li>src/dolphinui.rc <span style="color: grey">(f197af4)</span></li>
<li>src/kitemviews/kfileitemmodel.h <span style="color: grey">(8a0df72)</span></li>
<li>src/kitemviews/kfileitemmodel.cpp <span style="color: \
grey">(05f85a6)</span></li>
<li>src/tests/kfileitemmodeltest.cpp <span style="color: grey">(6c45e11)</span></li>
<li>src/views/dolphinview.h <span style="color: grey">(0b0d819)</span></li>
<li>src/views/dolphinview.cpp <span style="color: grey">(4105628)</span></li>
<li>src/views/dolphinviewactionhandler.h <span style="color: \
grey">(eb375b5)</span></li>
<li>src/views/dolphinviewactionhandler.cpp <span style="color: \
grey">(858f929)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129533/diff/" style="margin-left: \
3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments \
</h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/11/25/f02b7e85-d0b3-4153-ae0d-e86a4ddfc916__looptest.png">looptest</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/12/04/1fcc4412 \
-e5f2-4c31-92bf-b828eea938f7__contextdisabled.png">context-menu-disabled</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/12/04/8a4709e5 \
-d674-4f74-8cbc-a71841db7c36__selectioncontextenabled.png">context-menu-with-selection</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/12/04/1ddaa88d-e0aa-441a-a840-5e2cdc06fbf2__viewmenuenabled.png">view-menu-enabled</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============2381394263597058693==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic