D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

This revision was automatically updated to reflect the committed changes.
Closed by commit R118:727fd286bca4: [Cuttlefish] Overhaul program, use Kirigami 
(authored by cblack).

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66373=66409

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Menu.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/ResponsivePreview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/ToolsResponsive.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/iconmodel.cpp
  cuttlefish/src/iconmodel.h
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham, ndavis, ognarb
Cc: ognarb, GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, 
ngraham, plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Shipit!

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham, ndavis, ognarb
Cc: ognarb, GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, 
ngraham, plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack added inline comments.

INLINE COMMENTS

> ngraham wrote in Menu.qml:33
> Add icon

Seems to be a bug with the Qt.labs module, but this menu is just an extra 
(that's why it's loaded by a Loader instead of instantiated directly) so I 
wouldn't worry about it too much.

> ngraham wrote in Menu.qml:41
> Add icon

see above

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham, ndavis, ognarb
Cc: ognarb, GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, 
ngraham, plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66373.
cblack added a comment.


  Add copyright to files that I touched a lot

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66353=66373

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Menu.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/ResponsivePreview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/ToolsResponsive.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/iconmodel.cpp
  cuttlefish/src/iconmodel.h
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham, ndavis, ognarb
Cc: ognarb, GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, 
ngraham, plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carl Schwan]

ognarb accepted this revision.
ognarb added a comment.


  It's a huge improvement. You shouldn't forget to add your copyright to some 
files.

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham, ndavis, ognarb
Cc: ognarb, GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, 
ngraham, plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Noah Davis]

ndavis accepted this revision.
ndavis added a comment.


  It would be nice to have this in the stable release.

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham, ndavis
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  This is really excellent work. Let's get it in before 5.17 branches.

INLINE COMMENTS

> Menu.qml:33
> +}
> +MenuItem {
> +iconName: "edit-copy"

Add icon

> Menu.qml:41
> +}
> +MenuItem {
> +iconName: "document-open"

Add icon

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66353.
cblack added a comment.


  Background color of icons grid now uses view background color again

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66346=66353

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Menu.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/ResponsivePreview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/ToolsResponsive.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/iconmodel.cpp
  cuttlefish/src/iconmodel.h
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Should sort case-insensitively, so this doesn't happen: F7372225: 
Screenshot_20190917_173807.png 
  
  Also I still think the view background color for the scrollview would be good 
to bring back, so we don't have 100% of the UI using the window background color

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack added a comment.


  In D23712#533514 , @ngraham wrote:
  
  > I'm not sure it's working
  
  
  Should be working now

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66346.
cblack added a comment.


  Fix sorting, remove hardcoded icons

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66180=66346

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Menu.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/ResponsivePreview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/ToolsResponsive.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/iconmodel.cpp
  cuttlefish/src/iconmodel.h
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  I'm not sure it's working:
  
  F7370579: Screenshot_20190917_140415.png 


REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack added a comment.


  In D23712#532224 , @ngraham wrote:
  
  > Awesome.
  >
  > Didn't the main icon grid used to have a white background? I kind of liked 
that. Speaking of the icon grid, I'm somehow only not noticing that it's not 
sorted in any appreciable way. Maybe it should be alphabetical.
  
  
  Is the alphabetical sorting to your fancy?

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66180.
cblack added a comment.


  Alphabetical sort and more telling Qt's garbage SVG renderer to shut up

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66179=66180

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Menu.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/ResponsivePreview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/ToolsResponsive.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/iconmodel.cpp
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Awesome.
  
  Didn't the main icon grid used to have a white background? I kind of liked 
that. Speaking of the icon grid, I'm somehow only not noticing that it's not 
sorted in any appreciable way. Maybe it should be alphabetical.

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66179.
cblack added a comment.


  Shrink sidebar

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66178=66179

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Menu.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/ResponsivePreview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/ToolsResponsive.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Lovely stuff.
  
  The addition of a 256px icon in the sidebar makes it absurdly wide though. 
I'd recommend removing it.

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66178.
cblack added a comment.


  Spacing fixes

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66177=66178

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Menu.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/ResponsivePreview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/ToolsResponsive.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66177.
cblack added a comment.


  Whole buncha stuff

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66093=66177

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Menu.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/ResponsivePreview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/ToolsResponsive.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Noah Davis]

ndavis added a comment.


  There seems to be a blank entry at the bottom of the category list: F7357648: 
Screenshot_20190915_171648.png 

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Noah Davis]

ndavis added a comment.


  The clickable area is significantly smaller than the highlight.

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Noah Davis]

ndavis added a comment.


  Text still gets cut off on the left side: F7357632: 
Screenshot_20190915_171434.png 

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Noah Davis]

ndavis added a comment.


  The sidebar seems a bit wide. Maybe remove the 128px view? As a Breeze icon 
designer, I don't really need it.

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Noah Davis]

ndavis added a comment.


  The colorscheme selector does not work

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66093.
cblack added a comment.


  Clean up commented code, colorscheme -> color scheme

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66092=66093

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66092.
cblack added a comment.


  Wrap text, add custom message handler to get rid of Qt SVG renderer spam

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66089=66092

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Awesome, so close! We're just at little nitpick territory now.

INLINE COMMENTS

> IconGridDelegate.qml:69
>  
> -PlasmaComponents.Label {
> +QQC2.Label {
>  font.pointSize: iconSize > 96 ? theme.defaultFont.pointSize : 
> theme.smallestFont.pointSize

There's a lot of commented code in here that could also be cleaned up

> Preview.qml:254
> +QQC2.MenuItem {
> +text: i18n("Screenshot with Active Colorscheme")
> +onTriggered: {

Colorscheme -> Color Scheme

> SvgGrid.qml:92
>  opacity: iconGrid.currentIndex == index ? 1.0 : 0.7
>  anchors {
>  left: parent.left

Could also get rid of the commented code in here

> Tools.qml:111
> +QQC2.ComboBox {
> +model: ["System Colorscheme", "Breeze (Normal)", "Breeze Dark"]
> +delegate: QQC2.ItemDelegate {

System Colorscheme -> System Color Scheme

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Björn Feber]

GB_2 added inline comments.

INLINE COMMENTS

> Preview.qml:82
>  
> -Item { height: units.gridUnit / 2 }
> -
> -PlasmaExtras.Heading {
> -level: 3
> -elide: Text.ElideRight
> +Kirigami.Heading {
> +level: 1

Maybe add `wrapMode: Text.Wrap`, so the heading can have multiple lines.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66089.
cblack added a comment.


  Add a space

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66088=66089

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66088.
cblack added a comment.


  Shortcut fixemups

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66087=66088

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Björn Feber]

GB_2 added inline comments.

INLINE COMMENTS

> Tools.qml:108
> +QQC2.Label {
> +text: i18n("Colorscheme:")
>  }

"Color scheme"

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Sweet, once the Ctrl+Q shortcut is fixed (Maybe that's an upstream issue 
too?), the UI will get a thumbs-up from me!

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Definitely should be fixed upstream. Feel free to send a patch for that!

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66087.
cblack added a comment.


  Begone, modality and hello remembering sizes

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66070=66087

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack added a comment.


  In D23712#531391 , @ngraham wrote:
  
  > Whoa, I just noticed someehing really weird happening: when I click on any 
of the comboboxes in the toolbar, the whole UI now darkens except for the 
combobox's popup. This effect isn't done anywhere else and should be removed.
  
  
  That comes from the `org.kde.desktop` style setting the combobox's popup to 
be modal, which results in a scrim being added. I added an override here, but 
that should probably be fixed in the desktop style as well.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Oh another thing: now the window size doesn't get remembered across launches, 
which is super annoying due to the excessively small default size (worth fixing 
here IMO) that results in the sidebar taking up like 40% of the view.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Whoa, I just noticed someehing really weird happening: when I click on any of 
the comboboxes in the toolbar, the whole UI now darkens except for the 
combobox's popup. This effect isn't done anywhere else and should be removed.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack added a comment.


  In D23712#531076 , @ngraham wrote:
  
  > Ctrl+Q still doesn't work.
  
  
  QML shortcut type seems to not be firing regardless of where it's placed in 
the program.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66070.
cblack added a comment.


  Add global menu bar for users with a global menu bar.

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=66064=66070

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/GlobalMenuBar.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 66064.
cblack added a comment.


  Padding and focus fixes

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65929=66064

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Ctrl+Q still doesn't work.
  
  Found one more thing: the search field is no longer focused by default like 
it is with the current/old version.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> ngraham wrote in Tools.qml:36
> Set the same thing for the right margin too

Not done yet

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65929.
cblack added a comment.


  Substitute cuttlefish.close() for Qt.quit()

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65895=65929

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Ctrl+Q no longer quits the program now.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65895.
cblack added a comment.


  Passive notification on clipboard copy

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65894=65895

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65894.
cblack added a comment.


  Port to QQmlApplicationEngine

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65893=65894

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/CMakeLists.txt
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp
  cuttlefish/src/view.h

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65893.
cblack added a comment.


  Sidebar updates

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65878=65893

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Some general comments:
  
  - Try to use QML Layouts as much as possible; for the most part, everything 
should be in a layout.
  - Anytime you're tempted to create a `Rectangle` with `color: "transparent"`, 
don't. Just use an `Item` instead. But you may not even need to do this if you 
use Layouts more.
  
  Idea: since the sidebar is now nice and wide, don't put the icon name inside 
the squeezed `FormLayout` below the icon montage. Instead, make it a header 
above the icon montage. Here's a patch to your patch that will implement these 
and other proposed changes: https://invent.kde.org/snippets/443. It will look 
like this:
  
  F7341810: Screenshot_20190911_163718.png 


INLINE COMMENTS

> Preview.qml:78
>  anchors.fill: parent
> -spacing: units.gridUnit / 2
> +columns: 1
> +Rectangle {

A GridLayout with one column is just a ColumnLayout; use that instead

> Preview.qml:135
>  }
> -PlasmaCore.IconItem {
> -source: iconName
> -usesPlasmaTheme: cuttlefish.usesPlasmaTheme
> -colorGroup: PlasmaCore.ColorScope.colorGroup
> -Layout.preferredWidth: indexToSize(2)
> -Layout.preferredHeight: indexToSize(2)
> -}
> -PlasmaCore.IconItem {
> -source: iconName
> -usesPlasmaTheme: cuttlefish.usesPlasmaTheme
> -colorGroup: PlasmaCore.ColorScope.colorGroup
> -Layout.preferredWidth: indexToSize(3)
> -Layout.preferredHeight: indexToSize(3)
> -}
> -PlasmaCore.IconItem {
> -source: iconName
> -usesPlasmaTheme: cuttlefish.usesPlasmaTheme
> -colorGroup: PlasmaCore.ColorScope.colorGroup
> -Layout.preferredWidth: indexToSize(4)
> -Layout.preferredHeight: indexToSize(4)
> -}
> -}
> +QQC2.ToolButton {
> +visible: !iconPreview.screenshotting

Center this button horizontally

> Tools.qml:36
> +anchors.fill: parent
> +anchors.leftMargin: Kirigami.Units.smallSpacing
> +anchors.verticalCenter: parent.verticalCenter

Set the same thing for the right margin too

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65878.
cblack added a comment.


  Return of the sidebar

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65877=65878

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65877.
cblack added a comment.


  Wording change

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65876=65877

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65876.
cblack added a comment.


  Reorder menu items

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65875=65876

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65875.
cblack added a comment.


  Add option to take screenshot of both light/dark at once

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65874=65875

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/DualMontage.qml
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65874.
cblack added a comment.


  Add colorscheme changer

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65530=65874

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  One thing I just noticed is missing is the option to quickly switch the whole 
display to use the opposite color scheme (i.e. Breeze Dark if you're using 
default Breeze) The current version cuttlefish does this and I find it quite 
handy.
  
  It would also be nice if the built-in montage functionality had a feature to 
spit out a combined breeze light and breeze dark image so you didn't need to 
create both manually.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Björn Feber]

GB_2 added a comment.


  In D23712#526883 , @cblack wrote:
  
  > I preferred the sidebar as well, but a sidebar with the necessary width to 
fit the label + the largest icon name in Breeze without layout issues would be 
obscenely large, and would take over half the width of smaller monitors. That's 
why it was moved to the bottom - for extra horizontal space.
  
  
  What about enabling word wrap on the icon name and putting the 8-64 sizes 
above 128 like before?

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: GB_2, trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65530.
cblack added a comment.


  Add 22 size to preview, and hardcode sizes since they shouldn't need to 
respect any settings.

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65505=65530

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

To: cblack, #vdg, ngraham
Cc: trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack added a comment.


  In D23712#526852 , @ndavis wrote:
  
  > In D23712#526826 , 
@trickyricky26 wrote:
  >
  > > I love this patch, however I am curious as to why you moved the sidebar 
to the bottom.
  > >  Judging from your screenshot, it results in quite a bit of empty 
horizontal space.
  > >  Conceptually I think it is nicer to have the scrollable view go to the 
bottom of the window and have the details of the selected item be shown in a 
sidebar.
  >
  >
  > I agree. I also have a much easier time reading lists vertically rather 
than horizontally, so I prefer more vertical space at the cost of horizontal 
space.
  
  
  I preferred the sidebar as well, but a sidebar with the necessary width to 
fit the label + the largest icon name in Breeze without layout issues would be 
obscenely large, and would take over half the width of smaller monitors. That's 
why it was moved to the bottom - for extra horizontal space.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Noah Davis]

ndavis added a comment.


  In D23712#526826 , @trickyricky26 
wrote:
  
  > I love this patch, however I am curious as to why you moved the sidebar to 
the bottom.
  >  Judging from your screenshot, it results in quite a bit of empty 
horizontal space.
  >  Conceptually I think it is nicer to have the scrollable view go to the 
bottom of the window and have the details of the selected item be shown in a 
sidebar.
  
  
  I agree. I also have a much easier time reading lists vertically rather than 
horizontally, so I prefer more vertical space at the cost of horizontal space.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[TrickyRicky]

trickyricky26 added a comment.


  I love this patch, however I am curious as to why you moved the sidebar to 
the bottom.
  Judging from your screenshot, it results in quite a bit of empty horizontal 
space.
  Conceptually I think it is nicer to have the scrollable view go to the bottom 
of the window and have the details of the selected item be shown in a sidebar.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: trickyricky26, davidre, ndavis, filipf, davidedmundson, ngraham, 
plasma-devel, #vdg, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Just noticed something odd: the multi-size view shows an 8px version, but not 
the 22px version. Only one type of icon uses the 8px version (emblems) but 22px 
is more common. We should add that too.
  
  Also the last two slider values are still displayed incorrectly in the label.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: davidre, ndavis, filipf, davidedmundson, ngraham, plasma-devel, #vdg, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack marked 18 inline comments as done.
cblack added inline comments.

INLINE COMMENTS

> ngraham wrote in view.cpp:41
> Probably makes sense to reduce this to 600 if we can get the layout to adjust 
> adequately.

There's already icon names that are too long for the minimum 800 width window, 
so I wouldn't decrease it even further.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: davidre, ndavis, filipf, davidedmundson, ngraham, plasma-devel, #vdg, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65505.
cblack added a comment.


  Updates based off of feedback

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65483=65505

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

To: cblack, #vdg, ngraham
Cc: davidre, ndavis, filipf, davidedmundson, ngraham, plasma-devel, #vdg, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> view.cpp:41
> +setMinimumHeight(800);
> +setMinimumWidth(800);
> +

Probably makes sense to reduce this to 600 if we can get the layout to adjust 
adequately.

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: davidre, ndavis, filipf, davidedmundson, ngraham, plasma-devel, #vdg, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Better, thanks. The only remaining issue I can see is the problem with the 
slider values. I also noticed that the first value does nothing to change the 
view and spits out the following errors on the console:
  

file:///home/nate/kde/usr/share/plasma/packages/org.kde.plasma.cuttlefish/contents/ui/Tools.qml:84:
 Error: Cannot assign [undefined] to int

file:///home/nate/kde/usr/share/plasma/packages/org.kde.plasma.cuttlefish/contents/ui/Tools.qml:76:
 Error: Cannot assign [undefined] to QString

INLINE COMMENTS

> SvgGrid.qml:23
>  import QtQuick 2.2
> -// import QtQuick.Controls 1.0
> +import QtQuick.Controls 2.0
>  import QtQuick.Layouts 1.0

same: `import QtQuick.Controls 2.5 as QQC2`

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: davidre, ndavis, filipf, davidedmundson, ngraham, plasma-devel, #vdg, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65483.
cblack added a comment.


  Update based off of feedback

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65417=65483

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

To: cblack, #vdg, ngraham
Cc: davidre, ndavis, filipf, davidedmundson, ngraham, plasma-devel, #vdg, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[David Redondo]

davidre added a comment.


  In D23712#525974 , @ngraham wrote:
  
  > There are some inline comments that aren't done yet. Also, the slider's 
text label still says 81 and 162 instead of 64 and 128 for the last two slider 
values.
  
  
  I also noticed that the first two values are both say 16

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: davidre, ndavis, filipf, davidedmundson, ngraham, plasma-devel, #vdg, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[David Redondo]

davidre added inline comments.

INLINE COMMENTS

> cuttlefish.qml:32
>  
> -width: units.gridUnit * 60
> -height: Math.round(width / 3 * 2)
> -
>  property int iconSize: units.iconSizes.large
>  property bool darkScheme: false

Why not Kirigami units here?

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg, ngraham
Cc: davidre, ndavis, filipf, davidedmundson, ngraham, plasma-devel, #vdg, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  There are some inline comments that aren't done yet. Also, the slider's text 
label still says 81 and 162 instead of 64 and 128 for the last two slider 
values.

INLINE COMMENTS

> Tools.qml:102
> +color: Kirigami.Theme.textColor
> +height: 1
> +anchors {

Is any of this custom styling actually necessary given `Kirigami.Separator`'s 
default appearance?

> ngraham wrote in Tools.qml:30
> The fake toolbar created here feels too short. I would make it taller by a 
> few pixels so there's more room above and below the controls.

Now it's a bit too tall, and the margins above and below the search field still 
don't match the left margins: F7320966: Screenshot_20190905_071959.png 


> ngraham wrote in Tools.qml:42
> Don't need to override the default placeholder text when using 
> `Kirigami.SearchField`

Not done yet.

> ngraham wrote in cuttlefish.qml:24
> ditto

Not done

REPOSITORY
  R118 Plasma SDK

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

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

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65417.
cblack added a comment.


  Give screenie button some text

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65401=65417

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

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

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> Preview.qml:135
> +}
> +QQC2.ToolButton {
> +visible: !iconPreview.screenshotting

There's plenty of space for a button that has visible text. I'd recommend doing 
that instead of using a tiny icons-only button.

REPOSITORY
  R118 Plasma SDK

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

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

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65401.
cblack added a comment.


  Wrap those strings with i18n()

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65400=65401

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

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

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65400.
cblack marked an inline comment as done.
cblack added a comment.


  Add changes based off of feedback, screenshot functionality

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65356=65400

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml
  cuttlefish/src/main.cpp
  cuttlefish/src/view.cpp

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

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> Tools.qml:30
> +width: parent.width
> +RowLayout {
> +anchors.verticalCenter: parent.verticalCenter

The fake toolbar created here feels too short. I would make it taller by a few 
pixels so there's more room above and below the controls.

> Tools.qml:37
>  }
> -}
> -tools: RowLayout {
> -
> -spacing: units.gridUnit / 2
> -
> -PlasmaComponents.TextField {
> +Kirigami.SearchField {
>  id: filterInput

There's a bunch of unnecessary extra space to the left:

F7318225: Screenshot_20190904_102232.png 

REPOSITORY
  R118 Plasma SDK

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

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

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Nice!
  
  I notice that for some reason, the slider's last two values now display 81 
and 162 in the text field.
  
  Also while you're at it, could you make the icon grid alphabetically ordered? 
Right now I can't tell the sort order and it just looks random.

INLINE COMMENTS

> Preview.qml:83
>  Layout.fillWidth: true
> -onClicked: clipboard(preview.iconName);
> -}
> -PlasmaComponents.ToolButton {
> -text: pickerMode ?  i18n("Insert QtQuick code") : i18n("Copy 
> QtQuick code to clipboard")
> -iconSource: "edit-copy"
> -Layout.fillWidth: true
> -onClicked: {
> -var code = "/* Don't forget to import...\n\
> -import org.kde.plasma.core 2.0 as PlasmaCore\n\
> -*/\n\
> -PlasmaCore.IconItem {\n\
> -source: \"" + preview.iconName + "\"\n\
> -Layout.preferredWidth: units.iconSizes.medium\n\
> -Layout.preferredHeight: units.iconSizes.medium\n\
> -}\n\
> -";
> -clipboard(code);
> +GridLayout {
> +anchors.centerIn: parent

Use a Kirigami.FormLayout for this (if not using it was deliberate because of 
layout bugs, let's fix those)

> Preview.qml:91
> +}
> +ToolButton {
> +icon.name: "edit-copy"

Needs a tooltip saying that this will copy the name to the clipboard (the icon 
isn't enough)

> Preview.qml:94
> +text: preview.iconName
> +onClicked: clipboard(preview.iconName)
> +}

This is going to shock people, but I might even suggest displaying a Kirigami 
PassiveNotification saying " copied to clipboard" when this is clicked. 
Otherwise it looks like nothing happens when you click the button.

> Preview.qml:101
> +}
> +ToolButton {
> +icon.name: "document-open"

Needs a tooltip saying that this will open the image in an external app (the 
icon isn't enough, though making it actually use the icon of the app that it 
will open with would help with that)

> Tools.qml:23
>  import QtQuick 2.2
> -import QtQuick.Controls 2.0
> +import QtQuick.Controls 2.5
>  import QtQuick.Layouts 1.0

`import QtQuick.Controls 2.5 as QQC2` (See T10862 
)

> Tools.qml:94
>  
> -PlasmaComponents.TextField {
> +TextField {
>  id: pixelSizeInput

Let's just use a label for this. There's no value to letting people enter 
arbitrary numbers here.

> Tools.qml:114
> +}
> +Rectangle {
> +opacity: 0.2

You could potentially use a Kirigami.Separator for this

> cuttlefish.qml:24
>  import QtQuick.Layouts 1.0
> +import QtQuick.Controls 2.5
>  

ditto

REPOSITORY
  R118 Plasma SDK

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

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

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Filip Fila]

filipf added a comment.


  This is pretty great, the old version was buggy. One question: can we enforce 
a minimum width on the whole window?
  
  F7315330: image.png 
  
  The program's not very usable when the window is too narrow IMO.

INLINE COMMENTS

> cuttlefish.qml:33
> +height: Kirigami.Units.gridUnit * 50
> +width: Kirigami.Units.gridUnit * 50
>  

you can just do `width: height`

REPOSITORY
  R118 Plasma SDK

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

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

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack updated this revision to Diff 65356.
cblack added a comment.


  Removed checkbox for unused feature

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23712?vs=65355=65356

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml

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

D23712: [Cuttlefish] Overhaul program, use Kirigami

[David Edmundson]

davidedmundson added a comment.


  I love a patch with that much red.
  
  ++ to the concept, we shouldn't be using plasma outside of plasma.

INLINE COMMENTS

> Preview.qml:123
> -source: iconName
> -usesPlasmaTheme: cuttlefish.usesPlasmaTheme
> -colorGroup: PlasmaCore.ColorScope.colorGroup

This feature will be missing. Despite still having the checkbox for it.

REPOSITORY
  R118 Plasma SDK

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

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

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Nathaniel Graham]

ngraham added a comment.


  Sweet, will test and review soon.

INLINE COMMENTS

> Tools.qml:42
>  focus: true
> +placeholderText: i18n("Search icons...")
>  

Don't need to override the default placeholder text when using 
`Kirigami.SearchField`

REPOSITORY
  R118 Plasma SDK

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

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

D23712: [Cuttlefish] Overhaul program, use Kirigami

[Carson Black]

cblack created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
cblack requested review of this revision.

REVISION SUMMARY
  Cuttlefish's UI got an overhaul.
  
  - Won't break if window width is different than expected.
  - Plasma components have been replaced with Kirigami and QQC2 components
  - Preview moved to bottom of screen
  - Copy icon name button is now part of icon name instead of being an 
independent button
  - Copy code is removed
  - Change preview on hover is removed
  - Open icon file added
  - Other UI tweaks (spacing, colors)

REPOSITORY
  R118 Plasma SDK

BRANCH
  modernization (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/IconGrid.qml
  cuttlefish/package/contents/ui/IconGridDelegate.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/SvgGrid.qml
  cuttlefish/package/contents/ui/Tools.qml
  cuttlefish/package/contents/ui/cuttlefish.qml

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