D26983: Enable adding place as first child

gvgeo added a comment. In D26983#629925 , @ngraham wrote: > That's not how this works; reviewers being slow is not a good reason to abandon a patch. If you've lost interest in it, you can ask someone else can take it over, but just throwing

D28002: [WIP]Allow disabling of data collection

gvgeo abandoned this revision. gvgeo added a comment. Don't see any interest (or response), made a bug report instead. https://bugs.kde.org/show_bug.cgi?id=418981 REPOSITORY R849 User Feedback Collection Framework REVISION DETAIL https://phabricator.kde.org/D28002 To: gvgeo,

D26983: Enable adding place as first child

gvgeo abandoned this revision. gvgeo added a comment. Thanks for your help @meven, but I 'll close this patch now. It doesn't seem to be high in priorities. And to be honest among other reasons, after I found how problematic the code is, I'm not so keen on building on top of it.

D28002: [WIP]Allow disabling of data collection

gvgeo added a comment. In D28002#626663 , @davidedmundson wrote: > > Currently data is collected by default, with no way of disabling it. > > That's far from true. Things are off by default. Most providers are not run at all. > The

D28002: [WIP]Allow disabling of data collection

gvgeo added a comment. >> Currently data is collected by default, with no way of disabling it. > > That's far from true. Things are off by default. Most providers are not run at all. > The commit message is either confused or misleading. I made an assumption, based on the

D28002: [WIP]Allow disabling of data collection

gvgeo edited the summary of this revision. REPOSITORY R849 User Feedback Collection Framework REVISION DETAIL https://phabricator.kde.org/D28002 To: gvgeo, #frameworks, apol, vkrause Cc: davidedmundson, #frameworks

D28002: [WIP]Allow disabling of data collection

gvgeo added a subscriber: Frameworks. gvgeo added a comment. Added Frameworks for visibility. kuserfeedback is not included in the group. REPOSITORY R849 User Feedback Collection Framework REVISION DETAIL https://phabricator.kde.org/D28002 To: gvgeo Cc: #frameworks

D26983: Enable adding place as first child

gvgeo edited the summary of this revision. REPOSITORY R241 KIO BRANCH after (branched from master) REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure, ngraham Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, bruns

D26983: Enable adding place as first child

gvgeo added inline comments. INLINE COMMENTS > kfileplacesmodel.cpp:1059 > { > -KBookmark bookmark = KFilePlacesItem::createBookmark(d->bookmarkManager, > - text, url, iconName); > +KBookmark bookmark = KFilePlacesItem::createBookmark(d->bookmarkManager, > text,

D26983: Enable adding place as first child

gvgeo added a comment. ping #frameworks REPOSITORY R241 KIO BRANCH after (branched from master) REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack,

D26983: Enable adding place as first child

gvgeo edited the test plan for this revision. REPOSITORY R241 KIO BRANCH after (branched from master) REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D26983: Enable adding place as first child

gvgeo added a comment. ping REPOSITORY R241 KIO BRANCH after (branched from master) REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27296: [PlasmaComponents]Clean up ListItem code

This revision was automatically updated to reflect the committed changes. Closed by commit R242:10c112fccda8: [PlasmaComponents]Clean up ListItem code (authored by gvgeo). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27296?vs=75391=76411

D26983: Enable adding place as first child

gvgeo added a comment. After some digging, I found that KBookmarkGroup has more items that the KFilePlacesModel. It is impossible to make `KBookmark` move at the end of a `BookmarkGroup`, if the item is not in the `KFilePlacesModel`. I don't see a better solution than the current

D27083: [PC2]Give Label integer width.

gvgeo added a comment. In D27083#614285 , @ngraham wrote: > So the test needs to be fixed? In my opinion the Plasma Controls need fix. They need to always display the icons in an real position, no matter what their x position is. Just

D27296: [PlasmaComponents]Clean up ListItem code

gvgeo added a comment. The 2 removed lines were leftovers from 54e87fe879ba . REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D27296 To: gvgeo, #plasma Cc:

D27083: [PC2]Give Label integer width.

gvgeo abandoned this revision. gvgeo added a comment. The problem is with controls, not with label. QtQuick controls work fine. problem F8112471: Screenshot_20200219_124506.png F8112472: Screenshot_20200219_124602.png

D27083: [PC2]Give Label integer width.

gvgeo added a comment. > Also, why is only PC2 affected? Just checked again, apparently made a mistake. All the labels are affected. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D27083 To: gvgeo, #plasma, #vdg, ndavis, davidedmundson Cc:

D27083: [PC2]Give Label integer width.

gvgeo added a comment. Should I close this patch then? The only place I've seen this problem is in tests. I don't know if there is any place that is affected. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D27083 To: gvgeo, #plasma, #vdg,

D27083: [PC2]Give Label integer width.

gvgeo added a comment. I see, so paintedHeight is not exactly perfect above. > But I don't really understand how this helps anything in a reliable way. > Layouts will override widths after a layout invalidation. Correctly set anchors would override it. RowLayout, FlowLayout and

D27083: [PC2]Give Label integer width.

gvgeo added a comment. @davidedmundson ping REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D27083 To: gvgeo, #plasma, #vdg, ndavis, davidedmundson Cc: davidedmundson, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh,

D27083: [PC2]Give Label integer width.

gvgeo added a comment. While I was improving the test.qml, found that Headings also have the same problem but vertically. I imagine that is from the fractional-part in the multiplier. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D27083 To:

D27083: [PC2]Give Label integer width.

gvgeo edited the test plan for this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D27083 To: gvgeo, #plasma, #vdg, ndavis, davidedmundson Cc: davidedmundson, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh,

D27296: [PlasmaComponents]Clean up ListItem code

gvgeo created this revision. gvgeo added a reviewer: Plasma. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. gvgeo requested review of this revision. REVISION SUMMARY Removed two unused lines. Removed old comments. Minor style changes to comments. TEST

D27221: Make PC3 ToolButton use SmallMedium icon size by default, to match other versions

gvgeo added a comment. It could be my broken build. But this is what I see with X11 and scale 1.75. F8093437: Screenshot_20200209_152630.png F8093436: Screenshot_20200209_152557.png REPOSITORY R242 Plasma

D27221: Make PC3 ToolButton use SmallMedium icon size by default, to match other versions

gvgeo added a comment. On second thought, maybe it was fine. `Layout.preferredWidth` should not have any effect when icon is not visible anyway. And negative value could be set if needed. REPOSITORY R242 Plasma Framework (Library) BRANCH fix-pc3-toolbutton-sizing (branched from master)

D27221: Make PC3 ToolButton use SmallMedium icon size by default, to match other versions

gvgeo added inline comments. INLINE COMMENTS > ToolButton.qml:58 > Layout.fillHeight: control.icon.height <= 0 > Layout.preferredWidth: control.icon.width > 0 ? > control.icon.width : -1 > Layout.preferredHeight: control.icon.height > 0 ? >

D27082: [Tests]Make radiobutton3.qml use PC3

gvgeo added a comment. I 'll ask you to keep committing my patches a little longer. At least for my current patches, and their related bugfixes. REPOSITORY R242 Plasma Framework (Library) BRANCH radiobutton (branched from master) REVISION DETAIL https://phabricator.kde.org/D27082

D27082: [Tests]Make radiobutton3.qml use PC3

gvgeo added a comment. > Ship it! I can't do that, someone else need to commit it. REPOSITORY R242 Plasma Framework (Library) BRANCH radiobutton (branched from master) REVISION DETAIL https://phabricator.kde.org/D27082 To: gvgeo, #plasma, davidedmundson Cc: davidedmundson,

D27082: [Tests]Make radiobutton3.qml use PC3

gvgeo removed a reviewer: ndavis. gvgeo added a subscriber: ndavis. gvgeo added a comment. In D27082#604556 , @ndavis wrote: > poor horizontal alignment Don't make me redraw PC3 tests too. The basis of the issue is the same,

D27083: [PC2]Give Label integer width.

gvgeo added a comment. The Checkbox takes fractional position, caused from the fractional width of the Label. This lead to blurriness. F8071229: Screenshot_20200201_100353_2.png I don't understand what the problem is. Height can also take the

D26758: Fix the size and pixel alignment of checkboxes and radiobuttons

gvgeo added a comment. F8071207: alignment.png This shows the problem. Don't know what is the correct approach: 1 Label needs to be shorter, like PC3. 2 Give correct height in checkbox, and center the buttons. 3 Override label height in

D27083: [PC2]Give Label integer width.

gvgeo edited the test plan for this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D27083 To: gvgeo, #plasma, #vdg, ndavis Cc: kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen,

D27083: [PC2]Give Label integer width.

gvgeo added a comment. PC3 label gives integer size. If this patch is right, will make a patch to change all the tests to use PC2/PC3 label, instead of QQC2 for proper display. The 2 vertical misalignment problems are not the focus of this patch, created by: 1. PC2 label height

D27083: [PC2]Give Label integer width.

gvgeo edited the summary of this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D27083 To: gvgeo, #plasma, #vdg, ndavis Cc: kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, michaelh,

D27083: [PC2]Give Label integer width.

gvgeo edited the test plan for this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D27083 To: gvgeo, #plasma, #vdg, ndavis Cc: kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen,

D27082: [Tests]Make radiobutton3.qml use PC3

gvgeo retitled this revision from "Make radiobutton3.qml use PC3" to "[Tests]Make radiobutton3.qml use PC3". gvgeo edited the summary of this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D27082 To: gvgeo, #plasma, ndavis Cc:

D27083: [PC2]Give Label integer width.

gvgeo created this revision. gvgeo added reviewers: Plasma, VDG, ndavis. gvgeo added a project: Plasma. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. gvgeo requested review of this revision. REVISION SUMMARY Label paintedWidth creates alingment issues when

D27082: Make radiobutton3.qml use PC3

gvgeo added a comment. Because PC3 label is shorter that PC3 checkbox, there is a misalignment than need fixing. Either need to change label to have a height similar to PC2 (1.6*fontHeight). Or need to change this test's text vertical alignment. REPOSITORY R242 Plasma Framework (Library)

D27082: Make radiobutton3.qml use PC3

gvgeo created this revision. gvgeo added reviewers: Plasma, ndavis. gvgeo added a project: Plasma. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. gvgeo requested review of this revision. REVISION SUMMARY Correct radiobutton3.qml to use Plasma Components 3

D26983: Enable adding place as first child

gvgeo updated this revision to Diff 74749. gvgeo edited the test plan for this revision. gvgeo added a comment. This revision is now accepted and ready to land. The last index is incorrect, and I can't find a way to get the proper one. Restoring first revision, which keeps the functionality

D26983: Enable adding place as first child

gvgeo edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26983: Enable adding place as first child

gvgeo edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26983: Enable adding place as first child

gvgeo edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26983: Enable adding place as first child

gvgeo planned changes to this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26983: Enable adding place as first child

gvgeo updated this revision to Diff 74690. gvgeo added a comment. This revision is now accepted and ready to land. count -1 REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26983?vs=74689=74690 BRANCH after (branched from master) REVISION DETAIL

D26983: Enable adding place as first child

gvgeo planned changes to this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26983: Enable adding place as first child

gvgeo updated this revision to Diff 74689. gvgeo added a comment. Revert style edit REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26983?vs=74688=74689 BRANCH after (branched from master) REVISION DETAIL https://phabricator.kde.org/D26983 AFFECTED FILES

D26983: Enable adding place as first child

gvgeo marked 3 inline comments as done. REPOSITORY R241 KIO BRANCH after (branched from master) REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26983: Enable adding place as first child

gvgeo retitled this revision from "[WIP]Enable adding place as first child" to "Enable adding place as first child". REPOSITORY R241 KIO BRANCH after (branched from master) REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure Cc:

D26983: [WIP]Enable adding place as first child

gvgeo updated this revision to Diff 74688. gvgeo added a comment. Removed duplicate code, and used count instead. I was hopping for something better, this has couple more steps to complete. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D26983: [WIP]Enable adding place as first child

gvgeo added a dependent revision: D27032: Enable adding a place as first item.. REPOSITORY R241 KIO BRANCH after (branched from master) REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks, meven, broulik, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh,

D26758: Fix the size and pixel alignment of checkboxes and radiobuttons

gvgeo added a comment. @ndavis Any chance you missed my previous comment? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D26758 To: ndavis, #plasma, #vdg, ngraham Cc: gvgeo, ngraham, davidedmundson, kde-frameworks-devel, LeGast00n, GB_2,

D26983: [WIP]Enable adding place as first child

gvgeo added a comment. Although the target of this patch is the `else` part below, there is a need to add a place at the end. INLINE COMMENTS > kfileplacesmodel.cpp:1046 > { > -addPlace(text, url, iconName, appName, QModelIndex()); > +KBookmark bookmark =

D26983: [WIP]Enable adding place as first child

gvgeo edited the summary of this revision. gvgeo added a reviewer: Frameworks. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo, #frameworks Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26983: [WIP]Enable adding place as first child

gvgeo added a comment. Marked as WIP, cause I don't know how to avoid code duplication. Don't know what other places, that expect -1 to be last, need fix. Only have D26984 for dolphin, which is the reason I made this patch. REPOSITORY R241 KIO

D26983: [WIP]Enable adding place as first child

gvgeo added a dependent revision: D26984: Add a place, by drag into empty area of the panel. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26983 To: gvgeo Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26983: [WIP]Enable adding place as first child

gvgeo created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. gvgeo requested review of this revision. REVISION SUMMARY When index after is null, Move new bookmark into first place, by sending null in moveBookmark. TEST PLAN Before:

D26758: Fix the size and pixel alignment of checkboxes and radiobuttons

gvgeo added a comment. Plasma theme explorer is fine. The difference is that it uses plasmaComponents, while D26271 uses plasmaComponents**3**. About plasmaComponents: There is some bug with the checkbox and the Label. Fails to take the correct

D26735: [Components 3]Make slider tickmarks optional

gvgeo abandoned this revision. gvgeo added a comment. Closing, when there are few steps, should be marked. > ...should be a checkbox for you? That's good idea. I 'll try to add a checkbox when a keyboard is on/off only. Not sure how it would look. REPOSITORY R242 Plasma

D26735: [Components 3]Make slider tickmarks optional

gvgeo added a comment. My case would be solved with `stepCount > 1`. But AFAIK there are keyboards with 3-4 steps. F7889338: Screenshot_20200118_064621.png > Is there a way to achieve this with the existing qqc2 API? Not that I know. And

D26735: [Components 3]Make slider tickmarks optional

gvgeo retitled this revision from "[Components 3]Make Slider tickmarks optional" to "[Components 3]Make slider tickmarks optional". REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D26735 To: gvgeo, #plasma, #vdg Cc: kde-frameworks-devel, LeGast00n,

D26735: [Components 3]Make Slider tickmarks optional

gvgeo added a comment. Added to disable tickmarks, in battery's keyboard brightness control. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D26735 To: gvgeo, #plasma, #vdg Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26735: [Components 3]Make Slider tickmarks optional

gvgeo created this revision. gvgeo added reviewers: Plasma, VDG. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. gvgeo requested review of this revision. REVISION SUMMARY Add tickmarksEnabled variable. TEST PLAN Test with `tickmarksEnabled: false`

D26634: Correctly compute bottom margin for grid delegates with subtitles

gvgeo added a comment. In D26634#593595 , @ngraham wrote: > That's what you see with this patch? My apologies, I've made an error. REPOSITORY R296 KDeclarative REVISION DETAIL https://phabricator.kde.org/D26634 To: ngraham,

D26634: Correctly compute bottom margin for grid delegates with subtitles

gvgeo added a comment. Then, is it an option to create GridDelegateSubtitles.qml instead? As is now, without subtitles, I see everywhere squash shapes and bigger than necessary empty space. now vs before F7881612: slim empty.png In this

D26634: Correctly compute bottom margin for grid delegates with subtitles

gvgeo added inline comments. INLINE COMMENTS > GridDelegate.qml:81 > height: Kirigami.Settings.isMobile ? Math.round((delegate.width - > Kirigami.Units.gridUnit) / 1.6) > : > Math.min(delegate.GridView.view.implicitCellHeight -