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
