D22817: Port KRunner to Plasma Components 3

[Kai Uwe Broulik]

broulik added a comment.


  > To add the clear button, maybe you can use Kirigami.SearchField instead?
  
  Not sure if that can do Plasma styling?

INLINE COMMENTS

> apol wrote in RunCommand.qml:56
> The button's height is by default really big and krunner then looks rather 
> comical.
> Here we're making sure it all stays as big as the text field after all.

Then this is a regression from PC2 and should be fixed.
There's even a manual test for PC2 to address this, so PC3 should have the same 
behavior.
F7121480: Screenshot_20190730_221403.png 

> apol wrote in RunCommand.qml:226
> PC3 has no moving hightlight delegate, it's the delegate that highlights 
> itself.

But now you get two "highlights", one following the mouse, one following the 
keyboard.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, broulik
Cc: ngraham, ognarb, broulik, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22817: Port KRunner to Plasma Components 3

[Nathaniel Graham]

ngraham added a comment.


  In D22817#504049 , @ognarb wrote:
  
  > To add the clear button, maybe you can use Kirigami.SearchField instead?
  
  
  +1, IMO this makes more sense than manufacturing our own search field, 
especially since the current implementation results in a close button that has 
a toolbutton style hover effect, unlike other ones.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, broulik
Cc: ngraham, ognarb, broulik, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22817: Port KRunner to Plasma Components 3

[Aleix Pol Gonzalez]

apol updated this revision to Diff 62811.
apol marked an inline comment as done.
apol added a comment.


  cleanups

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22817?vs=62809=62811

BRANCH
  arcpatch-D22817

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

AFFECTED FILES
  lookandfeel/contents/runcommand/RunCommand.qml

To: apol, #plasma, broulik
Cc: ognarb, broulik, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D22817: Port KRunner to Plasma Components 3

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> broulik wrote in RunCommand.qml:56
> Why?

The button's height is by default really big and krunner then looks rather 
comical.
Here we're making sure it all stays as big as the text field after all.

> broulik wrote in RunCommand.qml:226
> Why remove this? It breaks highlighting of history items.

PC3 has no moving hightlight delegate, it's the delegate that highlights itself.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, broulik
Cc: ognarb, broulik, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D22817: Port KRunner to Plasma Components 3

[Aleix Pol Gonzalez]

apol updated this revision to Diff 62809.
apol added a comment.


  Have tooltips actually shown

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22817?vs=62772=62809

BRANCH
  arcpatch-D22817

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

AFFECTED FILES
  lookandfeel/contents/runcommand/RunCommand.qml

To: apol, #plasma, broulik
Cc: ognarb, broulik, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D22817: Port KRunner to Plasma Components 3

[Carl Schwan]

ognarb added a comment.


  To add the clear button, maybe you can use Kirigami.SearchField instead?

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, broulik
Cc: ognarb, broulik, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D22817: Port KRunner to Plasma Components 3

[Kai Uwe Broulik]

broulik added a comment.


  The padding on the left of the `TextField` looks off and what's up with that 
clear button?
  F7118082: Screenshot_20190730_084115.png 

  No tooltip shows up for the configure and close buttons.

INLINE COMMENTS

> RunCommand.qml:56
>  Layout.alignment: Qt.AlignTop
> +height: queryField.implicitHeight
> +

Why?

> RunCommand.qml:59
>  PlasmaComponents.ToolButton {
> -iconSource: "configure"
> +implicitHeight: queryField.implicitHeight
> +implicitWidth: queryField.implicitHeight

Why?

> RunCommand.qml:226
>  keyNavigationWraps: true
> -highlight: PlasmaComponents.Highlight {}
>  highlightMoveDuration: 0

Why remove this? It breaks highlighting of history items.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma
Cc: broulik, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22817: Port KRunner to Plasma Components 3

[Aleix Pol Gonzalez]

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

REVISION SUMMARY
  Using the new, stable components. Biggest change is that now we don't
  have a clear button and we need to add it by hand.

TEST PLAN
  Been using it for a few days, forgot to send the patch

REPOSITORY
  R120 Plasma Workspace

BRANCH
  krunner-pc3

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

AFFECTED FILES
  lookandfeel/contents/runcommand/RunCommand.qml

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