ervin marked 6 inline comments as done. ervin added inline comments. INLINE COMMENTS
> davidedmundson wrote in kconfigdialogmanager.cpp:609 > Why not item->readDefault()? Wouldn't do the same thing at all. readDefault() takes a KConfig object and updates the default value stored in the item with what it found in there. Yes, I know... the item API is weird... > davidedmundson wrote in kconfigdialogmanager.cpp:625 > won't it do it itself when the property changes? Good point, was indeed unnecessary now, I removed the line. > davidedmundson wrote in settingsstatusindicator.cpp:75 > > This is equivalent to calling showFullScreen(), showMaximized(), or > > setVisible(true), depending on the platform's default behavior for the > > window flags. > > For X and wayland it's setVisible(true) > > but we shouldn't count on it. I don't think it matters for widgets which have a parent and not the Qt::Window window flag, but OK, switching to setVisible() instead of show/hide. > davidedmundson wrote in settingsstatusindicator.cpp:175 > unused? I'm not sure how you end up to this conclusion, it's written two below if we are at the window edge, and it's used in the move call at the end. > davidedmundson wrote in settingsstatusindicator.cpp:184 > Can we be sure the tracked widget always has a parent widget? > > If someone doesn't use layouts a widget might not have a parent. Well that'd mean the tracked widget is a window... it's pretty much an impossibility IMO. > davidedmundson wrote in settingsstatusindicator.cpp:192 > that's not true for the RTL case where the widget is expected to resize. > > It would be w->pos().x() + w->width() - widgetExpectedWidth(w) Either I misunderstood what you meant or you got the math wrong on that one. What you're proposing (or what I understood you're proposing) breaks the "RTL + widget at edge" case in my tests. The line I wrote is working perfectly fine for my tests with desktoppath and qtquicksettings both in LTR and RTL modes. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis, broulik Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
