Add a TextField component
Needs RevisionPublic

Authored by ognarb on Sun, Feb 3, 11:30 PM.

Details

Reviewers
apol
ngraham
mart
Group Reviewers
Kirigami
Summary

<old>This is based on the searchField from Discover. It's a suggestion from @ngraham and I find it's a great idea.

Note: Because it's based on the searchField from Discover licensed under GPL2-or-later and Kirigami is licensed under LGPL2-or-later, this patch need the authorization from @apol and @ngraham, before landing.</old>

Now it's a generic TextField with the possibility to add a button left and a button right. Also, update the Sidebar example to use this component:

Test Plan

For now kirigami build. I wasn't only able to test this component by copying this component to the example directory.

Diff Detail

Repository
R169 Kirigami
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8047
Build 8065: arc lint + arc unit
ognarb created this revision.Sun, Feb 3, 11:30 PM
Restricted Application added a project: Kirigami. · View Herald TranscriptSun, Feb 3, 11:30 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ognarb requested review of this revision.Sun, Feb 3, 11:30 PM
ognarb edited the summary of this revision. (Show Details)Sun, Feb 3, 11:33 PM
ognarb added reviewers: apol, Kirigami, ngraham.
ognarb added a subscriber: apol.

Nice, I give my permission for the re-licensing.

src/controls/SearchField.qml
33 ↗(On Diff #50813)

@apol Is this still applicable if it's a re-usable component?

mart requested changes to this revision.Mon, Feb 4, 7:49 AM
mart added a subscriber: mart.

If we are going to have a specialized line edit, I think it shouldn't be specific for search, but be a common component of things that we have to boilerplate too often as they are missing from qqc2.
Namely, the clear button.
What I would like is a lineedit with two list properties leftactions and rightactions, which the clear button can be one of them (we have some fields around in plasma which have more than one action)

And then after that, maybe doing the specialization which is purely a search field

This revision now requires changes to proceed.Mon, Feb 4, 7:49 AM

I'm fine with having this code relicensed.

src/controls/SearchField.qml
33 ↗(On Diff #50813)

No, this is for Discover-specific behavior.

In D18716#404718, @mart wrote:

If we are going to have a specialized line edit, I think it shouldn't be specific for search, but be a common component of things that we have to boilerplate too often as they are missing from qqc2.
Namely, the clear button.
What I would like is a lineedit with two list properties leftactions and rightactions, which the clear button can be one of them (we have some fields around in plasma which have more than one action)

And then after that, maybe doing the specialization which is purely a search field

Interesting, but I have some question before I start implementing.

  • Do you have an example for a left action?
  • Should the buttons only be visible when the text is not empty? Or can this behavior also be configurable with a property?

Should the buttons only be visible when the text is not empty? Or can this behavior also be configurable with a property?

Action has a visible property which should be taken into account like normal here as well

I agree, it should be a generic TextField that just so happens to be useful as a search field if you'd like. Bonus points if one of the built-in actions is "show/hide password" so it can be used as a password field too.

mart added a comment.Wed, Feb 6, 3:41 PM

Interesting, but I have some question before I start implementing.

  • Do you have an example for a left action?

right now only web browsers url bars come to mind, so perhaps not important yet to add, (just to be tought of so can be added in the future should the need arise)

  • Should the buttons only be visible when the text is not empty? Or can this behavior also be configurable with a property?

should be configurable i think: a clear action should only be visible when there is text, instead a filter icons, or the show password action should probably be always there.

ognarb updated this revision to Diff 51215.Fri, Feb 8, 7:49 PM

Create a generic TextField instead

ognarb retitled this revision from Add a SearchField component to Add a TextField component.Fri, Feb 8, 7:54 PM
ognarb edited the summary of this revision. (Show Details)
ognarb edited the test plan for this revision. (Show Details)
ognarb edited the summary of this revision. (Show Details)Sat, Feb 9, 12:29 AM
ognarb edited the summary of this revision. (Show Details)
ognarb added inline comments.Sat, Feb 9, 12:33 AM
src/controls/TextField.qml
101

@mart I'm not sure if it's the right way to convert the icon from the action to the one needed by the toolButton (don't support icon.source)

src/kirigamiplugin.cpp
181

@mart Should I use 2.6 or 2.7? I didn't find any information about that version I should use.

mart requested changes to this revision.Sat, Feb 9, 10:09 AM
mart added inline comments.
src/controls/TextField.qml
74

I would like those to actually be lists as we do have use cases for more than one icon

94

this should be a repeater with the toolbutton as aproperty (maybe shouldn't even be a toolbotton, as usually they're just icons without any button graphics)

101

you can try to use a private class.
PrivateActionToolButton {
kirigamiAction: the action
background: null
}
that should already manage everything about the action (and with background:null to disable any kind of button background

This revision now requires changes to proceed.Sat, Feb 9, 10:09 AM
mart added a comment.Thu, Feb 14, 10:19 AM

also, the name should possibily not conflict with QQC2 TextField, like ActionTextField or something like that.

src/controls/TextField.qml
21

always namespace the controls import as
import QtQuick.Controls 2.1 as Controls

89

the style already sets the correct values for duration and timeout, this line should be removed

106

text will go under both buttons, in order to avoid that, probably anchors should be added to the TextField's contentItem to make space for the buttons.

mart added inline comments.Thu, Feb 14, 10:41 AM
src/controls/TextField.qml
106

err, TextField is not a Control, right.

so, it should set leftPadding and rightPadding to the space of the buttons

@mart thanks for the review, I don't have enough time this weekend, but I will start working on it next Tuesday :)