D8641: FormLayout

This revision was automatically updated to reflect the committed changes. Closed by commit R169:1a136c28f44c: FormLayout (authored by mart). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D8641?vs=23092&id=23317#toc REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricato

D8641: FormLayout

davidedmundson accepted this revision. davidedmundson added a comment. This revision is now accepted and ready to land. Please don't push new API 5 days before a release tagging. We'll still be able to make the Plasma 5.12 if we wait till Saturday. INLINE COMMENTS > FormLayout.qml:200-20

D8641: FormLayout

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in FormLayout.qml:152-153 > > why are we doing something different for width and height indeed the width should have the same logic, which now does,the width simply didn't trigger visible layout bugs REPOSITORY R169 Kirigami

D8641: FormLayout

mart updated this revision to Diff 23092. mart added a comment. - same logic for width REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8641?vs=23088&id=23092 BRANCH mart/formlayout REVISION DETAIL https://phabricator.kde.org/D8641 AFFECTED FILES ex

D8641: FormLayout

davidedmundson added inline comments. INLINE COMMENTS > mart wrote in FormLayout.qml:152-153 > I made now "prefer" preferredheight to implicitheight, but sometimes > layout.preferredheight is not set, and it needs to fallback to implicitheight > hor the layout will look completely broken > wh

D8641: FormLayout

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in FormLayout.qml:113 > Is this done? no, will do a related patch to breeze > davidedmundson wrote in FormLayout.qml:121 > I think you'll need to try again. > > Have you tested this code with a really really long label? a real

D8641: FormLayout

mart updated this revision to Diff 23088. mart added a comment. - just use item.Layout.preferredHeight - prefer Layout.preferredHeight to implicitHeight REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8641?vs=23075&id=23088 BRANCH mart/formlayout REVI

D8641: FormLayout

davidedmundson requested changes to this revision. davidedmundson added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > davidedmundson wrote in FormLayout.qml:113 > Flat mode: > > main page of current mouse KCM: > button order, icons. Is this done? > mart wrot

D8641: FormLayout

hein accepted this revision. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8641 To: mart, #plasma, #kirigami, hein, davidedmundson Cc: broulik, colomar, ngraham, davidedmundson, plasma-devel, ngiannip, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abet

D8641: FormLayout

mart added inline comments. INLINE COMMENTS > hein wrote in FormLayout.qml:66 > Should it be centralized or ...? I don't think so, reasons for switching to one or the other are different. this one is purely wether the contents fit or not, the global one for the app which is how many columns are

D8641: FormLayout

mart updated this revision to Diff 23075. mart added a comment. - hasOwnProperty REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8641?vs=23008&id=23075 BRANCH mart/formlayout REVISION DETAIL https://phabricator.kde.org/D8641 AFFECTED FILES examples

D8641: FormLayout

hein added inline comments. INLINE COMMENTS > mart wrote in FormLayout.qml:66 > yes, they are in some controls, independent from each other Should it be centralized or ...? > mart wrote in FormLayout.qml:114 > what's x in y? I meant the "in" operator, or alternatively object.hasOwnProperty("fo

D8641: FormLayout

mart added inline comments. INLINE COMMENTS > hein wrote in FormLayout.qml:66 > Doesn't Kirigami have wideMode logic somewhere else too? yes, they are in some controls, independent from each other > hein wrote in FormLayout.qml:114 > Isn't this going to cause tons of ReferenceErrors without "x

D8641: FormLayout

mart marked 6 inline comments as done. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8641 To: mart, #plasma, #kirigami, hein, davidedmundson Cc: broulik, colomar, ngraham, davidedmundson, plasma-devel, ngiannip, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuter

D8641: FormLayout

hein added a comment. In general it looks quite good to me. INLINE COMMENTS > FormLayout.qml:66 > + */ > +property bool wideMode: width >= lay.wideImplicitWidth > + Doesn't Kirigami have wideMode logic somewhere else too? > FormLayout.qml:114 > +//NOTE: this is a

D8641: FormLayout

This revision was automatically updated to reflect the committed changes. Closed by commit R169:adf62671e39e: adress small issues (authored by mart). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D8641?vs=22997&id=23006#toc REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://p

D8641: FormLayout

mart updated this revision to Diff 23008. mart added a comment. - an early prototype of a FormLayout - Form->FormData - add gallery page - make FormLayout a template - use Headings for all the titles - remove some redundant code - support for dynamic elements - add heuristic to ex

D8641: FormLayout

mart reopened this revision. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8641 To: mart, #plasma, #kirigami, hein, davidedmundson Cc: broulik, colomar, ngraham, davidedmundson, plasma-devel, ngiannip, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abet

D8641: FormLayout

mart updated this revision to Diff 22997. mart marked 14 inline comments as done. mart added a comment. - an early prototype of a FormLayout - Form->FormData - add gallery page - make FormLayout a template - use Headings for all the titles - remove some redundant code - support for

D8641: FormLayout

davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. Still has comments from my original review not replied to. Maybe using the "done" feature in phab, will make it eaiser to track things? Also this needs API d

D8641: FormLayout

mart reopened this revision. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8641 To: mart, #plasma, #kirigami, hein Cc: broulik, colomar, ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, he

D8641: FormLayout

This revision was automatically updated to reflect the committed changes. Closed by commit R169:90b11b50a68b: adress comments (authored by mart). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D8641?vs=22468&id=22894#toc REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabr

D8641: FormLayout

hein added a dependent revision: D8911: Port launch feedback KCM to Qt Quick and implement VDG redesign. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8641 To: mart, #plasma, #kirigami, hein Cc: broulik, colomar, ngraham, davidedmundson, plasma-devel, ZrenBot, progw

D8641: FormLayout

mart added inline comments. INLINE COMMENTS > broulik wrote in FormLayout.qml:45 > Use `Qt.callLater` (new in Qt 5.8) if you pass it an actual function (ie. not > an inline one) it will compress subsequent calls to one: > https://doc.qt.io/qt-5/qml-qtqml-qt.html#callLater-method > > There's pr

D8641: FormLayout

mart updated this revision to Diff 22468. mart marked 10 inline comments as done. mart added a comment. - adress comments REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8641?vs=22462&id=22468 BRANCH mart/formlayout REVISION DETAIL https://phabricator

D8641: FormLayout

mart updated this revision to Diff 22462. mart added a comment. - if there is a rendercontrol, watch the target window REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8641?vs=22410&id=22462 BRANCH mart/formlayout REVISION DETAIL https://phabricator.kd

D8641: FormLayout

broulik added inline comments. INLINE COMMENTS > FormLayoutGallery.qml:79 > +text: item ? "Remove Field" : "Add Field" > +property var item > +onClicked: { `property Item`? That's in a couple of places > FormLayout.qml:45 > + > +Timer { > +

D8641: FormLayout

mart updated this revision to Diff 22410. mart added a comment. - && means we want & and not a mnemonic REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8641?vs=22401&id=22410 BRANCH mart/formlayout REVISION DETAIL https://phabricator.kde.org/D8641 AF

D8641: FormLayout

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in mnemonicattached.cpp:49 > Note that the alt to show is a Breeze special feature for widgets. i think it's fine assuming the style supports that... (and assume that the one which is really supported is breeze) i wouldn't know

D8641: FormLayout

mart updated this revision to Diff 22401. mart added a comment. - mnemoinictext property, which always has the & - support mnemonics in the global drawer - different weigths for differnt control types - filter on the window not the app REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDAT

D8641: FormLayout

mart updated this revision to Diff 22345. mart added a comment. - partial support for weights - make the item with more weigth "win" REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8641?vs=22324&id=22345 BRANCH mart/formlayout REVISION DETAIL https:

D8641: FormLayout

davidedmundson added a comment. I like the idea of the attached mnemonic handler, it's good that you've kept it separate from the form layout. > i'll look at the algorythm to see if is reusable Can we try to avoid having a diff that keeps growing and growing with features. We'll e

D8641: FormLayout

mart added a comment. In https://phabricator.kde.org/D8641#167459, @hein wrote: > So if I understand correctly, the Mnemonic stuff currently doesn't seem to have the same aims as KAcceleratorManager does ... > > KAcceleratorManager acts on a hierarchy of widgets, traverses it, assigns

D8641: FormLayout

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in FormLayout.qml:121 > In general with text, I think it's better to set the item to occupy the whole > space it /can/ take up, then use the Text.Alignment property to position the > text within it. > > Otherwise if this wraps,

D8641: FormLayout

mart updated this revision to Diff 22324. mart added a comment. - an early prototype of a FormLayout - Form->FormData - add gallery page - make FormLayout a template - use Headings for all the titles - remove some redundant code - support for dynamic elements - add heuristic to ex

D8641: FormLayout

mart reopened this revision. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8641 To: mart, #plasma, #kirigami, hein Cc: colomar, ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, hein

D8641: FormLayout

This revision was automatically updated to reflect the committed changes. Closed by commit R169:8a1e6fc77848: Revert "text 100% wide" (authored by mart). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D8641?vs=22277&id=22322#toc REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE http

D8641: FormLayout

hein added a comment. So if I understand correctly, the Mnemonic stuff currently doesn't seem to have the same aims as KAcceleratorManager does ... KAcceleratorManager acts on a hierarchy of widgets, traverses it, assigns weights to actionable things and possible mnemonics and overall sm

D8641: FormLayout

mart updated this revision to Diff 22277. mart added a comment. - text 100% wide REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8641?vs=22274&id=22277 BRANCH mart/formlayout REVISION DETAIL https://phabricator.kde.org/D8641 AFFECTED FILES examples

D8641: FormLayout

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in FormLayout.qml:113 > I'd like it if we can make this on the desktop style look the same as > QGroupBox looks in flat mode. Even if that means changing our QStyle. > > Currently this means spanning both columns and then centre

D8641: FormLayout

mart updated this revision to Diff 22274. mart added a comment. - event compress relayouts - resize items that are separator - new way to switch from wide to compact - remove debug - introduce a mnemonics mechanism - split mnemonics in a new attached REPOSITORY R169 Kirigami CHANG

D8641: FormLayout

mart updated this revision to Diff 22033. mart added a comment. - add heuristic to exclude repeaters from layout REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8641?vs=21991&id=22033 BRANCH mart/formlayout REVISION DETAIL https://phabricator.kde.org/

D8641: FormLayout

davidedmundson added inline comments. INLINE COMMENTS > FormLayout.qml:71 > +itemContainer.parent = lay; > +//if section, wee need another placeholder > +if (item.Kirigami.FormData.isSection) { typo > FormLayout.qml:85-86 > +property var item > +

D8641: FormLayout

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in FormLayout.qml:8 > Why do we inherit from Control? I tend to have them controls by default as they have the padding properties that can be very convenient sometimes (as well, if one needs to put a background on the back of i

D8641: FormLayout

mart updated this revision to Diff 21991. mart added a comment. - add gallery page - make FormLayout a template - use Headings for all the titles - remove some redundant code - support for dynamic elements REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.

D8641: FormLayout

colomar added a comment. In https://phabricator.kde.org/D8641#164016, @ngraham wrote: > Do we have top have that giant banner on top of the sidebar list? This review request is about the form layout you see on the right, the sidebar has nothing to do with it. It's just in the scre

D8641: FormLayout

ngraham added a comment. Do we have top have that giant banner on top of the sidebar list? REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8641 To: mart, #plasma, #kirigami, hein Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed

D8641: FormLayout

davidedmundson added inline comments. INLINE COMMENTS > FormLayout.qml:8 > + > +Control { > +id: root Why do we inherit from Control? > FormLayout.qml:20 > +columnSpacing: Kirigami.Units.smallSpacing > +anchors { > +left: parent.left this can be taller than the

D8641: FormLayout

mart edited the summary of this revision. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8641 To: mart, #plasma, #kirigami, hein Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, davidedmundson, mart, hein

D8641: FormLayout

mart added a comment. F5470164: Spectacle.TJ9696.png REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8641 To: mart, #plasma, #kirigami, hein Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts

D8641: FormLayout

mart added a reviewer: hein. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8641 To: mart, #plasma, #kirigami, hein Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, davidedmundson, mart, hein

D8641: FormLayout

mart created this revision. mart added reviewers: Plasma, Kirigami. Restricted Application added projects: Plasma, Kirigami. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY Add a new component called FormLayout, which aims to behave similar to Qwidget's QFormLayout: