D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Ivan Čukić 
ivan added a comment.


  > used Layouts instead of Anchors
  
  I know, I'm oldschool - when I was young, we didn't have QML, Kirigami and 
... :)

REPOSITORY
  R119 Plasma Desktop

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

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Ivan Čukić 
This revision was automatically updated to reflect the committed changes.
Closed by commit R119:36e322792002: Add the option to show the current activity 
name and icon (authored by ivan).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28136?vs=78018=78021

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

AFFECTED FILES
  applets/showActivityManager/package/contents/config/config.qml
  applets/showActivityManager/package/contents/config/main.xml
  applets/showActivityManager/package/contents/ui/ConfigAppearance.qml
  applets/showActivityManager/package/contents/ui/main.qml

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Nathaniel Graham 
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Thanks!
  
  I'd still prefer if main.qml used Layouts instead of Anchors, I also still 
think the option to show a generic icon is not really very useful; I have a 
hard time envisioning a user who manually sets icons for their activities but 
prefers to show a generic icon in the pager. However I don't use activities and 
it's your code, so your call. :)

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

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Ivan Čukić 
ivan added a comment.


  Last change - use "activities" icon if no icon is set (though this should not 
happen if the user creates the activity through legal ways)

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

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Ivan Čukić 
ivan updated this revision to Diff 78018.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28136?vs=78014=78018

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

AFFECTED FILES
  applets/showActivityManager/package/contents/config/config.qml
  applets/showActivityManager/package/contents/config/main.xml
  applets/showActivityManager/package/contents/ui/ConfigAppearance.qml
  applets/showActivityManager/package/contents/ui/main.qml

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Ivan Čukić 
ivan marked 3 inline comments as done.
ivan added a comment.


  Done

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

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Ivan Čukić 
ivan updated this revision to Diff 78014.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28136?vs=78006=78014

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

AFFECTED FILES
  applets/showActivityManager/package/contents/config/config.qml
  applets/showActivityManager/package/contents/config/main.xml
  applets/showActivityManager/package/contents/ui/ConfigAppearance.qml
  applets/showActivityManager/package/contents/ui/main.qml

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Nathaniel Graham 
ngraham added inline comments.

INLINE COMMENTS

> ConfigAppearance.qml:48
> +}
> +
> +CheckBox {

I would add some whitespace between these, since they're different logical 
groups. You can easily do this with the following:

`item { Kirigami.FormData.isSection: true }`

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

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Ivan Čukić 
ivan added a comment.


  Kirigami has quite interesting features I must say :)

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

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Ivan Čukić 
ivan updated this revision to Diff 78006.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28136?vs=77985=78006

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

AFFECTED FILES
  applets/showActivityManager/package/contents/config/config.qml
  applets/showActivityManager/package/contents/config/main.xml
  applets/showActivityManager/package/contents/ui/ConfigAppearance.qml
  applets/showActivityManager/package/contents/ui/main.qml

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Ivan Čukić 
ivan added a comment.


  The reason for the icon setting is to have full backwards compatibility. 
Maybe someone likes the static icon better than random icons they set for each 
activity. I don't  like to have everything configurable, but here it is not a 
significant maintainance overhead, and the configuration dialogue would be 
quite dull with a single checkbox :)

REPOSITORY
  R119 Plasma Desktop

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

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Nathaniel Graham 
ngraham added a comment.


  Pretty nice! I wonder if we even need an option for the activity option 
though. How about this: if the activity has an icon, always show it. If it 
doesn't, show the generic one. That would probably satisfy everyone 
automatically, by re-using the user's preference with respect to icons for 
their activities.
  
  Also in general it's nice to see Layouts used in new code as it tends to 
substantially simplify the width and height code for individual items.

INLINE COMMENTS

> ConfigAppearance.qml:36
> +Label {
> +text: i18n("Icon:")
> +}

this should instead be a `Kirigami.FormData.label: `i18n("Icon:")` property set 
on `radioCurrentActivityIcon`

> ConfigAppearance.qml:53
> +Label {
> +text: i18n("Title:");
> +}

this should instead be a `Kirigami.FormData.label: `i18n("Title:")` property 
set on `checkShowActivityName`

REPOSITORY
  R119 Plasma Desktop

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

To: ivan, #plasma, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Ivan Čukić 
ivan added a comment.


  Screenshot:
  
  F8184234: Screenshot_20200319_112003.png 


REPOSITORY
  R119 Plasma Desktop

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

To: ivan, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

2020-03-19 Thread Ivan Čukić 
ivan created this revision.
ivan added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ivan requested review of this revision.

REVISION SUMMARY
  The current activity switching applet shows a generic icon that
  opens the activity switcher which does not offer any information
  on the current activity. This does not go well with other applets
  like desktop pager.
  
  This patch adds options (on by default) to show the current activity
  icon and name in the applet.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  ivan/activity-switcher-applet-name-and-icon

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

AFFECTED FILES
  applets/showActivityManager/package/contents/config/config.qml
  applets/showActivityManager/package/contents/config/main.xml
  applets/showActivityManager/package/contents/ui/ConfigAppearance.qml
  applets/showActivityManager/package/contents/ui/main.qml

To: ivan, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart