D26021: Display sub categories when searching

2019-12-20 Thread Marco Martin 
This revision was automatically updated to reflect the committed changes.
Closed by commit R124:f82cca0cb07f: Display sub categories when searching 
(authored by mart).

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26021?vs=71889=71890

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

AFFECTED FILES
  CMakeLists.txt
  core/MenuModel.cpp
  core/MenuModel.h
  core/MenuProxyModel.cpp
  sidebar/CMakeLists.txt
  sidebar/SidebarMode.cpp
  sidebar/SidebarMode.h
  sidebar/ToolTips/tooltipmanager.cpp
  sidebar/ToolTips/tooltipmanager.h
  sidebar/package/contents/ui/CategoriesPage.qml
  sidebar/package/contents/ui/IntroIcon.qml
  sidebar/package/contents/ui/SubCategoryPage.qml
  sidebar/package/contents/ui/main.qml

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

D26021: Display sub categories when searching

2019-12-20 Thread Marco Martin 
mart updated this revision to Diff 71889.
mart added a comment.


  - root categories are 0 depth

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26021?vs=71881=71889

BRANCH
  arcpatch-D26021

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

AFFECTED FILES
  CMakeLists.txt
  core/MenuModel.cpp
  core/MenuModel.h
  core/MenuProxyModel.cpp
  sidebar/CMakeLists.txt
  sidebar/SidebarMode.cpp
  sidebar/SidebarMode.h
  sidebar/ToolTips/tooltipmanager.cpp
  sidebar/ToolTips/tooltipmanager.h
  sidebar/package/contents/ui/CategoriesPage.qml
  sidebar/package/contents/ui/IntroIcon.qml
  sidebar/package/contents/ui/SubCategoryPage.qml
  sidebar/package/contents/ui/main.qml

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

D26021: Display sub categories when searching

2019-12-20 Thread Marco Martin 
mart added inline comments.

INLINE COMMENTS

> davidedmundson wrote in MenuModel.cpp:49
> In other models we tend to drop the suffix "Role" from the rolename
> 
> i.e Qt::DisplayRole -> "display"
> 
> but it's not super important.

I followed what it was doing previously, MenuItemRols->MenuItem would clash 
with the class MenuItem

REPOSITORY
  R124 System Settings

BRANCH
  arcpatch-D26021

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

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

D26021: Display sub categories when searching

2019-12-20 Thread David Edmundson 
davidedmundson accepted this revision.
davidedmundson added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> MenuModel.cpp:49
>  
> +QHash MenuModel::roleNames() const
> +{

In other models we tend to drop the suffix "Role" from the rolename

i.e Qt::DisplayRole -> "display"

but it's not super important.

> MenuModel.cpp:124
> +MenuItem *candidate = mi;
> +int depth = 0;
> +while ( candidate && candidate->parent() ) {

I find it un-intuitive that they effectively start at 1, not 0.

The hidden root object is 0, but that can't be accessed via an index.

It took me a while to understand why the delegate spacing logic  had depth-2

REPOSITORY
  R124 System Settings

BRANCH
  arcpatch-D26021

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

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

D26021: Display sub categories when searching

2019-12-20 Thread Marco Martin 
mart updated this revision to Diff 71881.
mart added a comment.


  - remove trailing space

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26021?vs=71880=71881

BRANCH
  arcpatch-D26021

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

AFFECTED FILES
  CMakeLists.txt
  core/MenuModel.cpp
  core/MenuModel.h
  core/MenuProxyModel.cpp
  sidebar/CMakeLists.txt
  sidebar/SidebarMode.cpp
  sidebar/SidebarMode.h
  sidebar/ToolTips/tooltipmanager.cpp
  sidebar/ToolTips/tooltipmanager.h
  sidebar/package/contents/ui/CategoriesPage.qml
  sidebar/package/contents/ui/IntroIcon.qml
  sidebar/package/contents/ui/SubCategoryPage.qml
  sidebar/package/contents/ui/main.qml

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

D26021: Display sub categories when searching

2019-12-20 Thread Marco Martin 
mart updated this revision to Diff 71880.
mart added a comment.


  - switch column when there are two

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26021?vs=71876=71880

BRANCH
  arcpatch-D26021

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

AFFECTED FILES
  CMakeLists.txt
  core/MenuModel.cpp
  core/MenuModel.h
  core/MenuProxyModel.cpp
  sidebar/CMakeLists.txt
  sidebar/SidebarMode.cpp
  sidebar/SidebarMode.h
  sidebar/ToolTips/tooltipmanager.cpp
  sidebar/ToolTips/tooltipmanager.h
  sidebar/package/contents/ui/CategoriesPage.qml
  sidebar/package/contents/ui/IntroIcon.qml
  sidebar/package/contents/ui/SubCategoryPage.qml
  sidebar/package/contents/ui/main.qml

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

D26021: Display sub categories when searching

2019-12-20 Thread Marco Martin 
mart updated this revision to Diff 71876.
mart added a comment.


  - don't accept invalid index as root
  - select proper category when not in search more

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26021?vs=71803=71876

BRANCH
  arcpatch-D26021

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

AFFECTED FILES
  CMakeLists.txt
  core/MenuModel.cpp
  core/MenuModel.h
  core/MenuProxyModel.cpp
  sidebar/CMakeLists.txt
  sidebar/SidebarMode.cpp
  sidebar/SidebarMode.h
  sidebar/ToolTips/tooltipmanager.cpp
  sidebar/ToolTips/tooltipmanager.h
  sidebar/package/contents/ui/CategoriesPage.qml
  sidebar/package/contents/ui/IntroIcon.qml
  sidebar/package/contents/ui/SubCategoryPage.qml
  sidebar/package/contents/ui/main.qml

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

D26021: Display sub categories when searching

2019-12-18 Thread Marco Martin 
mart updated this revision to Diff 71803.
mart added a comment.


  - Pleased code enter the commit message for your changes. Lines starting

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26021?vs=71799=71803

BRANCH
  arcpatch-D26021

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

AFFECTED FILES
  CMakeLists.txt
  core/MenuModel.cpp
  core/MenuModel.h
  sidebar/CMakeLists.txt
  sidebar/SidebarMode.cpp
  sidebar/SidebarMode.h
  sidebar/ToolTips/tooltipmanager.cpp
  sidebar/ToolTips/tooltipmanager.h
  sidebar/package/contents/ui/CategoriesPage.qml
  sidebar/package/contents/ui/IntroIcon.qml
  sidebar/package/contents/ui/SubCategoryPage.qml
  sidebar/package/contents/ui/main.qml

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

D26021: Display sub categories when searching

2019-12-18 Thread Nathaniel Graham 
ngraham added inline comments.

INLINE COMMENTS

> MenuModel.cpp:25
>  #include 
> -
> +#include 
>  #include "MenuItem.h"

Remove

> CategoriesPage.qml:149
> +//icon: model.decoration
> +//label: model.display
>  separatorVisible: false

Remove

REPOSITORY
  R124 System Settings

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

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

D26021: Display sub categories when searching

2019-12-18 Thread Marco Martin 
mart added a comment.


  In D26021#579771 , @ngraham wrote:
  
  > Nice!
  >
  > Somehow this has regressed the text color for the selected item: F7824037: 
Screenshot_20191218_064325.png 
  >
  > It used to be white before this patch.
  
  
  new revision should be fine now

REPOSITORY
  R124 System Settings

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

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

D26021: Display sub categories when searching

2019-12-18 Thread Marco Martin 
mart updated this revision to Diff 71799.
mart added a comment.


  - white text when selected
  - lways load the module when in search mode

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26021?vs=71776=71799

BRANCH
  arcpatch-D26021

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

AFFECTED FILES
  CMakeLists.txt
  core/MenuModel.cpp
  core/MenuModel.h
  sidebar/CMakeLists.txt
  sidebar/SidebarMode.cpp
  sidebar/SidebarMode.h
  sidebar/ToolTips/tooltipmanager.cpp
  sidebar/ToolTips/tooltipmanager.h
  sidebar/package/contents/ui/CategoriesPage.qml
  sidebar/package/contents/ui/IntroIcon.qml
  sidebar/package/contents/ui/SubCategoryPage.qml
  sidebar/package/contents/ui/main.qml

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

D26021: Display sub categories when searching

2019-12-18 Thread Marco Martin 
mart added a comment.


  In D26021#579771 , @ngraham wrote:
  
  > Nice!
  >
  > Somehow this has regressed the text color for the selected item: F7824037: 
Screenshot_20191218_064325.png 
  >
  > It used to be white before this patch.
  
  
  ah, right, i had to redo the delegate.. a moment..

REPOSITORY
  R124 System Settings

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

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

D26021: Display sub categories when searching

2019-12-18 Thread Nathaniel Graham 
ngraham added a comment.


  Nice!
  
  Somehow this has regressed the text color for the selected item: F7824037: 
Screenshot_20191218_064325.png 
  
  It used to be white before this patch.

REPOSITORY
  R124 System Settings

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

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

D26021: Display sub categories when searching

2019-12-18 Thread Marco Martin 
mart updated this revision to Diff 71776.
mart added a comment.


  - get rid of the const cast

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26021?vs=71775=71776

BRANCH
  arcpatch-D26021

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

AFFECTED FILES
  CMakeLists.txt
  core/MenuModel.cpp
  core/MenuModel.h
  sidebar/CMakeLists.txt
  sidebar/SidebarMode.cpp
  sidebar/SidebarMode.h
  sidebar/ToolTips/tooltipmanager.cpp
  sidebar/ToolTips/tooltipmanager.h
  sidebar/package/contents/ui/CategoriesPage.qml
  sidebar/package/contents/ui/IntroIcon.qml
  sidebar/package/contents/ui/SubCategoryPage.qml
  sidebar/package/contents/ui/main.qml

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

D26021: Display sub categories when searching

2019-12-18 Thread Marco Martin 
mart marked 2 inline comments as done.
mart added inline comments.

INLINE COMMENTS

> davidedmundson wrote in SidebarMode.cpp:400
> please check validity, otherwise model is null and we'll crash.
> 
> also ..why make it const, only to cast it?

got rif of the cast

REPOSITORY
  R124 System Settings

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

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

D26021: Display sub categories when searching

2019-12-18 Thread Marco Martin 
mart added inline comments.

INLINE COMMENTS

> davidedmundson wrote in SidebarMode.cpp:400
> please check validity, otherwise model is null and we'll crash.
> 
> also ..why make it const, only to cast it?

QModelIndex::index() is always const?

REPOSITORY
  R124 System Settings

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

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

D26021: Display sub categories when searching

2019-12-18 Thread Marco Martin 
mart updated this revision to Diff 71775.
mart marked 3 inline comments as done.
mart added a comment.


  - adress comments
  - fix clicking againthe same category

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26021?vs=71614=71775

BRANCH
  arcpatch-D26021

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

AFFECTED FILES
  CMakeLists.txt
  core/MenuModel.cpp
  core/MenuModel.h
  sidebar/CMakeLists.txt
  sidebar/SidebarMode.cpp
  sidebar/SidebarMode.h
  sidebar/ToolTips/tooltipmanager.cpp
  sidebar/ToolTips/tooltipmanager.h
  sidebar/package/contents/ui/CategoriesPage.qml
  sidebar/package/contents/ui/IntroIcon.qml
  sidebar/package/contents/ui/SubCategoryPage.qml
  sidebar/package/contents/ui/main.qml

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

D26021: Display sub categories when searching

2019-12-18 Thread David Edmundson 
davidedmundson added inline comments.

INLINE COMMENTS

> MenuModel.cpp:102
> +MenuItem *candidate = mi->parent();
> +while ( candidate && candidate->parent() && 
> candidate->parent()->parent() ) {
> +candidate = candidate->parent();

More comments are needed throughout.

You forgot why we went to the second level of heirachy only 10 minutes after 
coding it when I asked :P

> MenuModel.cpp:108
>  }
> +//if (mi->parent())  theData.setValue( mi->parent()->name() );
>  break;

can be deleted

> SidebarMode.cpp:263
>  KActionCollection *collection = nullptr;
> -QPersistentModelIndex activeCategoryIndex;
> -int activeCategory;
> -int activeSubCategory;
> +QPersistentModelIndex activeCategoryRowIndex;
> +int activeCategoryRow = -1;

calling an int categoryRow makes sense, as the int refers to the row.

Calling an index CategoryRow doesn't.

> SidebarMode.cpp:400
>  if (showToolTips()) {
> -
> d->subCategoryToolTipManager->requestToolTip(d->subCategoryModel->index(index,
>  0), rect.toRect());
> +d->toolTipManager->setModel(const_cast *>(index.model()));
> +d->toolTipManager->requestToolTip(index, rect.toRect());

please check validity, otherwise model is null and we'll crash.

also ..why make it const, only to cast it?

REPOSITORY
  R124 System Settings

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

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