> On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
> > libs/taskmanager/taskactions.cpp, line 135
> > <http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line135>
> >
> >     why a property for this? why not simply:
> >     
> >     if (!act->toolTip().isEmpty()) {

Using "if (!act->toolTip().isEmpty())" results in the menuitem's text being 
shown as the tooltip - which is pretty silly.

I've also tried "if(!act->toolTip().isEmpty() && act->toolTip()!=act->text())" 
but this again fails if the menuitem has an underscore shortcut.


> On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
> > libs/taskmanager/taskactions.cpp, line 384
> > <http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line384>
> >
> >     we have a name, it should be used. it hardly matters what the eventual 
> > Launcher is called. shortening it with the removal of "It Is" is probably 
> > alright ("computer application english" :)

"Show A Launcher for Systemsettings When It is Not Running" is a *very* long 
menu item. What advantage does adding the name here give? The user already 
right-clicked on the taskbar entry - so they know what the item is. Placing its 
name here just adds extra noise. In fact I'd actually prefer just "Pin To 
Taskbar" and "Unpin From Taskbar"


> On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
> > libs/taskmanager/taskactions.cpp, line 415
> > <http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line415>
> >
> >     how well does this work with KUniqueApplications?

The only issue I've noticed is when Amarok is minimized to the system tray, and 
you click on its launcher. Then the spinner is re-shown.


> On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
> > libs/taskmanager/taskactions.cpp, lines 508-510
> > <http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line508>
> >
> >     there's a line between "simplifiying" and "making it useless in too 
> > many cases" that this crosses imho. this would have me reaching for the 
> > window titlebar in too many cases.
> >     
> >     i would be in favour of renaming "Advanced" (a word we are trying to 
> > get away from) with something like "More actions"

hehe! knew this would get some reactions. Its only an option anyway. Perhaps it 
would be better if the user could supply a mask of items they did *not* want in 
the basic menu? Then it's up to the taskbar applet to decide what to show?


> On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
> > libs/taskmanager/taskactions.cpp, lines 530-540
> > <http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line530>
> >
> >     with this change one has to open the goup, right click on the correct 
> > entry and then choose. i don't see the point of this simlification other 
> > than to make it harder to get at.

This is similar to the above issue. And would be solved by allowing the applet 
to explicitly set which items to *not* show. But I'm now not 100% convinced by 
this particular change...


> On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
> > libs/taskmanager/taskactions.cpp, lines 543-546
> > <http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line543>
> >
> >     no, this is one of the PRIMARY uses of the context menu. what would be 
> > nice is to move "To Current Desktop" into the DesktopsMenu, however, right 
> > above "All Desktops"

OK, I agree here.


> On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
> > libs/taskmanager/taskactions.cpp, lines 550-552
> > <http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line550>
> >
> >     could be moved into the "Advanced"(/"More Actions") menu?

But then it is different to the kwin menu, even in the non-basic mode.


> On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
> > libs/taskmanager/taskactions.cpp, line 600
> > <http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line600>
> >
> >     this is a misuse of QAction. you can add QActions to a QAction and that 
> > parent QAction becomes the submenu. no need for such custom property 
> > setting/reading and menu creation.

This was for dock-manager plugin support - just to make things easier on my 
side - then there's a 1:1 mapping between the dockmanager item and the QAction. 
No biggie, I can always remove this.


- Craig


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


On Oct. 31, 2011, 8:40 p.m., Craig Drummond wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103004/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2011, 8:40 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> 1. Add NewInstance action to launch a new instance of an application.
> 2. Move toggle launcher action out of advanced menu.
> 3. Don't show application name in 'Show A Launcher' action, as might not know 
> the real name at this time (e.g. no desktop file read).
> 4. Add application actions, to be shown at the top of the right-click menu. 
> IconTasks uses this to show recent docuents, dock manager menu items, and 
> unity items.
> 5. A ToolTipMenu class is created, so that tooltips may be give for menu 
> items. This is so that IconTasks can display the full path of a documents in 
> the recent documents menu.
> 6. Add option to have only basic window controls in menu.
> 
> 
> Diffs
> -----
> 
>   libs/taskmanager/taskactions.h 2b5a641 
>   libs/taskmanager/taskactions.cpp 0e6ba8e 
>   libs/taskmanager/taskactions_p.h 913966c 
> 
> Diff: http://git.reviewboard.kde.org/r/103004/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Craig Drummond
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to