Review Request 109966: Don't try to change the icon of the "sort" action menu

[Frank Reininghaus]

> On April 11, 2013, 2:32 p.m., Frank Reininghaus wrote:
> > Thanks for the patch!
> > 
> > I see that the code in question has been added in http://quickgit.kde.org/?p=kde-baseapps.git&a=commit&h=76258ddc9c3cb037b70ea2a060608ae4e397ae88 which originates in a review request by Todd: https://svn.reviewboard.kde.org/r/3862/
> > 
> > @Todd: can you comment on this?
> 
> Daniel Faust wrote:
>     Any news? KDE 4.10.3 will be tagged soon.
> 
> Emmanuel Pescosta wrote:
>     Maybe add Todd as reviewer?

The reply from Todd is below.

@Todd: Thanks for the clarification. I think the "better approach" makes sense.
@Daniel: can you update your patch according to Todd's suggestion?

---

Someone changed the code compared to my version.  It originally
checked whether the icon was present and only changed it if an icon is
present.  Now it doesn't do the check, hence the problem reported.  I
don't know who made the change or why.

The point of showing the icon is so the user can tell at a glance what
sorting behavior is currently being used.  I don't think removing that
is good for users.

A better approach would be to check if the icon for the menu has been
manually-specified, and if so use that, otherwise it should use the
icon for the current sorting method.

Part of the problem is that the sorting methods don't have default
icons for them yet.  If they did then none of this would be a problem.

Whatever the case, the same changes should probably be made to the view menu.


- Frank


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109966/#review30927
-----------------------------------------------------------


On April 11, 2013, 1:40 p.m., Daniel Faust wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109966/
> -----------------------------------------------------------
> 
> (Updated April 11, 2013, 1:40 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> When changing the sorting through the "sort" action menu, dolphin will try to change the icon.
> However this change fails and just removes the icon.
> 
> In my opinion there is no need at all to change the icon, it should always stay the same (it can be changed by the user with the kde tool bar editor btw.).
> 
> 
> This addresses bug 255819.
>     http://bugs.kde.org/show_bug.cgi?id=255819
> 
> 
> Diffs
> -----
> 
>   dolphin/src/views/dolphinviewactionhandler.cpp c7832d7 
> 
> Diff: http://git.reviewboard.kde.org/r/109966/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Daniel Faust
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130507/6e77e6b3/attachment.htm>