D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham abandoned this revision. ngraham added a comment. Don't need this; not the right approach. The original motivation for doing this (to work around the broken placeholder text in the PC2 TextField) is being fixed in the PC2 TextField with D15021 and

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham added a comment. Ah, I'm sorry. I'm getting my patches confused. The PC2 TextField's bug is that it doesn't show placeholder text properly with light themes when software rendering isn't being used. I'm open to other ways of fixing that. REPOSITORY R242 Plasma Framework (Library)

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

davidedmundson added a comment. > Placeholder text looks good with fractional scale factors (PC3 and QQC2 ones have this, PC2 and QQC1 ones do not) No, this is where we have crossed wires. PC3, QQC2 and PC2 look good. We control the renderType. Only QQC1 desktop theme has the

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham added a comment. To back up a bit, the issue is that we currently have no TextField with has the following characteristics: 1. Has built-in functionality for clear and password reveal buttons, so each client doesn't need to re-invent the wheel (PC2 one has this, PC3, QQC1, and

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

davidedmundson added a comment. > Upstream the features we wrote into our PlasmaComponents TextField and then just use that. Downsides: lengthy process, will take forever before we can actually use it here. Yes, but there's also no real rush from a PC POV. > Finally fix the Qt bug

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham added a comment. So from in-person conversations this week, it seems like we have a few paths forward here: - Upstream the features we wrote into our PlasmaComponents TextField and then just use that. **Downsides**: lengthy process, will take forever before we can actually use

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham added a comment. @davidedmundson ping: In D14345#297340 , @ngraham wrote: > Thanks for the history! Two questions: > > 1. Does this mean that PlasmaComponents is semi or fully deprecated or "legacy", and we should be porting

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham added a comment. I feel like the whole point of having these Plasma-ish wrappers is because they have extra features not present in the base Qt classes. If we just use QQC2 `TextField`s everywhere, then every one that wants a clear button (for example) needs to implement it for

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

safaalfulaij added a comment. In D14345#297340 , @ngraham wrote: > 1. Does this mean that PlasmaComponents is semi or fully deprecated or "legacy", and we should be porting Plasma stuff to Kirigami instead? > 2. Since there's no Kirigami

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

safaalfulaij added a comment. Great! Now I know why the port to PC3 didn't start :D Now to the patch. First, by this we will allow the user to get text under the icon, which will be impossible to read. Second is what I'm here for :) The clear button placement and icon should not follow

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham added a comment. Thanks for the history! Two questions: 1. Does this mean that PlasmaComponents is semi or fully deprecated or "legacy", and we should be porting Plasma stuff to Kirigami instead? 2. Since there's no Kirigami `TextField`, what do we do with this patch? Is there

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

davidedmundson added a comment. Story time! First we had PlasmaComponents (based on some Nokia crap and some custom things from late Plasma4) Then came along Plasma 5 PC2 which was a continuation of above - Then QQC1 came along plasmastyle which was a

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham added a comment. Oh. Maybe I don't know the history here. Why do we have PC2 and PC3 versions of this control, with the PC2 version using QQC1, and the PC3 version using QQC2, but not having all the features of the PC2 one? And why can't we change the PC3 version at all? REPOSITORY

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

davidedmundson added a comment. > Hmm QQC2 not PC2 REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D14345 To: ngraham, #plasma, mart, davidedmundson Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham added a comment. In D14345#297329 , @davidedmundson wrote: > PC3 was previously designed to be a drop in QQC2 implementation. There is currently no new API. > It /might/ have changed after we made plasmastyle instead. Check with

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. PC3 was previously designed to be a drop in QQC2 implementation. There is currently no new API. It /might/ have changed after we made plasmastyle instead. Check

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham edited the test plan for this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D14345 To: ngraham, #plasma, mart, davidedmundson Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham edited the summary of this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D14345 To: ngraham, #plasma, mart, davidedmundson Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

ngraham created this revision. ngraham added reviewers: Plasma, mart, davidedmundson. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. ngraham requested review of this revision. REVISION SUMMARY The PlasmaComponents2