[prev in list] [next in list] [prev in thread] [next in thread]
List: kfm-devel
Subject: Re: Review Request 109015: Cleanup Places Panel context menus
From: "Emmanuel Pescosta" <emmanuelpescosta099 () gmail ! com>
Date: 2015-12-18 10:18:26
Message-ID: 20151218101826.5237.72704 () mimi ! kde ! org
[Download RAW message or body]
--===============0666765274663502278==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On Dec. 15, 2015, 10:51 p.m., Kai Uwe Broulik wrote:
> > Ping
> >
> > Can we re-evaluate this, please? Right-clicking in the otherwise beautiful \
> > Dolphin makes me cringe even more nowadays.
>
> Thomas Pfeiffer wrote:
> Are there any open questions for the usability side?
>
> Emmanuel Pescosta wrote:
> > Can we re-evaluate this, please?
>
> Yes for sure :)
>
> Let us summarize the discussion a little bit:
> 1. Removing the device name in all the entries (Frank)
> 2. It might not be obvious that these options exist at all if the Panel is "almost \
> full" (Christoph) 3. I think clicking the section headings would still be okay \
> (Thomas) 4. Big "usability diff" between the Places Panel and KFilePlacesView \
> (Frank) 5. Dolphin placesview should replace current placeview in frameworks \
> (Àlex)
> I agree with 1, maybe we should also remove the context menu header from the \
> patched version?
> 2 and 3 are valid points, maybe we should add an info dialog, that hidden places \
> can still be shown via rightclick on the section headings or empty panel + show \
> hidden places, when the user hides a place for the first time.
>
> 4 should be easy to fix ;)
>
> About 5: I don't think that merging our places panel code into frameworks makes \
> sense, because the places panel uses Dolphin's own item model.
>
> Kai Uwe Broulik wrote:
> Thomas has told me many times that headings in context menus are bad, so yes, I'll \
> remove them. :)
> Good idea, Emmanuel, I like that a lot, perhaps we can use the message widget at \
> the top of the view to say "You can show hidden Places by right clicking the \
> headings or an empty area in the Places panel.", Thomas, I'm open for suggestions \
> about better wording.
> 4 should be easy but in the kdelibs discussion there were even more concerns than \
> in this RR...
> 4 should be easy but in the kdelibs discussion there were even more concerns than \
> in this RR...
Oh sorry I didn't know this, wasn't subscribed to the kdelibs ML back then :(
What were the main concerns?
> perhaps we can use the message widget at the top of the view
It should be possible for the user to permanently suppress such messages IMHO (aka \
don't show it again), which isn't supported by the message widget atm as far as I \
know.
- Emmanuel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/109015/#review89564
-----------------------------------------------------------
On Feb. 18, 2013, 7:43 p.m., Kai Uwe Broulik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/109015/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2013, 7:43 p.m.)
>
>
> Review request for Dolphin, KDE Usability and Frank Reininghaus.
>
>
> Bugs: 310764
> http://bugs.kde.org/show_bug.cgi?id=310764
>
>
> Repository: kde-baseapps
>
>
> Description
> -------
>
> This patch cleans up the places panel context menus by:
> - Removing the device name in all the entries, it just distracts and every … \
> single … entry has them. (It is debateable whether that title is needed or \
> fitting, I am not a huge fan of these)
> - Moving "Open in a new tab" to the top like in most other places in KDE (Unmount \
> and Empty Trash are always above because they're more important)
> - Moving Icon Size to the global context menu rather than the item context menu \
> where it made no sense from a context perspective
> - Removed "Unlock panels" from item context menu (also no "contextual" thing)
> - Removed "Add Entry" from item context menu
> - Removed "Show all entries" from item context menu
>
> See screenshot for direct comparison between all changed context menus.
> Of course this makes Dolphin's places panel drift even further away from generic \
> kdelibs places panel but they so different already …
>
> Diffs
> -----
>
> dolphin/src/panels/places/placesitemmodel.cpp 1acbb57
> dolphin/src/panels/places/placespanel.cpp e23732c
>
> Diff: https://git.reviewboard.kde.org/r/109015/diff/
>
>
> Testing
> -------
>
> Editing still works, hiding/showing items, showing all, changing icon size, \
> emptying trash, mounting/unmounting.
>
> File Attachments
> ----------------
>
> Comparison (left old, right new)
> https://git.reviewboard.kde.org/media/uploaded/files/2013/02/18/cleanupplaces.png
>
>
> Thanks,
>
> Kai Uwe Broulik
>
>
--===============0666765274663502278==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<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/109015/">https://git.reviewboard.kde.org/r/109015/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On December 15th, 2015, 10:51 p.m. CET, <b>Kai Uwe \
Broulik</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;">Ping</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Can we re-evaluate \
this, please? Right-clicking in the otherwise beautiful Dolphin makes me cringe even \
more nowadays.</p></pre> </blockquote>
<p>On December 15th, 2015, 11:03 p.m. CET, <b>Thomas Pfeiffer</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;">Are \
there any open questions for the usability side?</p></pre> </blockquote>
<p>On December 18th, 2015, 10:46 a.m. CET, <b>Emmanuel Pescosta</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;"><blockquote \
style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid \
#bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;"> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Can we re-evaluate this, please?</p> </blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Yes for sure :)</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Let \
us summarize the discussion a little bit: 1. Removing the device name in all the \
entries (Frank) 2. It might not be obvious that these options exist at all if the \
Panel is "almost full" (Christoph) 3. I think clicking the section headings would \
still be okay (Thomas) 4. Big "usability diff" between the Places Panel and \
KFilePlacesView (Frank) 5. Dolphin placesview should replace current placeview in \
frameworks (Àlex)</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">I agree with 1, maybe we should also \
remove the context menu header from the patched version?</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">2 and \
3 are valid points, maybe we should add an info dialog, that hidden places can still \
be shown via rightclick on the section headings or empty panel + show hidden places, \
when the user hides a place for the first time.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">4 should be easy to fix ;)</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">About \
5: I don't think that merging our places panel code into frameworks makes sense, \
because the places panel uses Dolphin's own item model.</p></pre>
</blockquote>
<p>On December 18th, 2015, 11:05 a.m. CET, <b>Kai Uwe Broulik</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;">Thomas has told me many times that headings in context menus are bad, so \
yes, I'll remove them. :)</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Good idea, Emmanuel, I like that a lot, \
perhaps we can use the message widget at the top of the view to say "You can show \
hidden Places by right clicking the headings or an empty area in the Places panel.", \
Thomas, I'm open for suggestions about better wording.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">4 \
should be easy but in the kdelibs discussion there were even more concerns than in \
this RR...</p></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;"><blockquote \
style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid \
#bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;"> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">4 should be easy but in the kdelibs discussion there were even more \
concerns than in this RR...</p> </blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Oh sorry I didn't know this, wasn't subscribed to the \
kdelibs ML back then :( What were the main concerns?</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid \
#bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;"> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">perhaps we can use the message widget at the top of the view</p> \
</blockquote> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">It should be possible for the user to permanently \
suppress such messages IMHO (aka don't show it again), which isn't supported by the \
message widget atm as far as I know.</p></pre> <br />
<p>- Emmanuel</p>
<br />
<p>On February 18th, 2013, 7:43 p.m. CET, Kai Uwe Broulik 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, KDE Usability and Frank Reininghaus.</div>
<div>By Kai Uwe Broulik.</div>
<p style="color: grey;"><i>Updated Feb. 18, 2013, 7:43 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="http://bugs.kde.org/show_bug.cgi?id=310764">310764</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-baseapps
</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;">This patch cleans up the places panel context menus by:
- Removing the device name in all the entries, it just distracts and every … \
single … entry has them. (It is debateable whether that title is needed or fitting, \
I am not a huge fan of these)
- Moving "Open in a new tab" to the top like in most other places in KDE \
(Unmount and Empty Trash are always above because they're more \
important)
- Moving Icon Size to the global context menu rather than the item context menu \
where it made no sense from a context perspective
- Removed "Unlock panels" from item context menu (also no \
"contextual" thing)
- Removed "Add Entry" from item context menu
- Removed "Show all entries" from item context menu
See screenshot for direct comparison between all changed context menus.
Of course this makes Dolphin's places panel drift even further away from generic \
kdelibs places panel but they so different already …</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;">Editing still works, hiding/showing items, showing all, changing icon \
size, emptying trash, mounting/unmounting.</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>dolphin/src/panels/places/placesitemmodel.cpp <span style="color: \
grey">(1acbb57)</span></li>
<li>dolphin/src/panels/places/placespanel.cpp <span style="color: \
grey">(e23732c)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/109015/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/2013/02/18/cleanupplaces.png">Comparison \
(left old, right new)</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============0666765274663502278==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic