D19606: [Task Manager] Reorganize and improve presentation of context menu

[Kai Uwe Broulik]

broulik added a comment.


  Overall I must say I'm quite a fan.
  
  I don't like the Settings and Alternatives entries at the top, they draw most attention to them for actions that should rather be least intrusive.
  I would prefer the jump list actions stuff at the top but grouping them with the other window/application actions is an interesting approach.
  While I personally don't use the Minimize/Maximize actions in the menu I've seen people get quite passionate about them.
  
  Did you test how media controls behave in this scenario? Also please make sure keyboard accelerators (Alt key, the `&` stuff) are sane.
  
  Also keep in mind to adjust KWin's title bar menu to use the new icons as well.

INLINE COMMENTS

> ContextMenu.qml:99
>          var lists = [
> -            backend.jumpListActions(launcherUrl, menu),
> -            backend.placesActions(launcherUrl, showAllPlaces, menu),
> -            backend.recentDocumentActions(launcherUrl, menu)
> +            [i18n("Places"), backend.placesActions(launcherUrl, showAllPlaces, menu)],
> +            [i18n("Recent Documents"), backend.recentDocumentActions(launcherUrl, menu)],

Please use a JS object for that instead of using just numbered indices

  var list = [
      {title: i18n("Places"), actions: backend. ...},
      {title: i18n("Recent Documents"), actions: ...}
      ...
  ];

> ContextMenu.qml:112
> +            // This section with the one beneath it that shows universal actions
> +            if (list[1].length > 0 || list[0] == i18n("Actions")) {
> +                var sectionHeader = newMenuItem(menu);

Comparing translated strings is usually bad style, please use my idea above, and then perhaps add a third `group: "actions"` or similar string for identification

> ContextMenu.qml:119
> +
> +            for (var key in list)
> +            for (var i = 0; i < list[1].length; ++i) {

Remove

> ContextMenu.qml:320
>          text: i18n("Move To &Desktop")
> +        icon: "go-next"
>  

I don't see the point of this icon, it adds visual noise for no better identification

> ContextMenu.qml:711
>  
> -        text: i18nc("Remove launcher button for application shown while it is not running", "Unpin")
> +        text: i18nc("Remove launcher button for application shown while it is not running", "Unpin from %1", plasmoid.title)
> +        icon: "window-pin"

Perhaps add some context explaining %1 is plasmoid title?

However, I can already see this becoming quite awful in German in case of icon tasks "An Fensterleiste nur mit Symbolen anheften" :/

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D19606

To: ngraham, #plasma, #vdg, ndavis
Cc: broulik, ndavis, trickyricky26, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190308/247112b0/attachment-0001.html>