D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ngraham added a comment. T13008: Figure out a good UI for the "show which settings have been changed" feature REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: ervin, ngraham, davidedmundson, meven, crossi, bport,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ngraham added a comment. Let's have the rest of the discussion in a central phab patch, which I probably should have pushed for at the start. I'll make one... REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: ervin, ngraham, davidedmundson, meven,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ndavis added a comment. I'm sorry this happened. I was working with what I could see. For what it's worth, I would have accepted it if you said it could not be done well. I know I don't know as much about the technical side of this as you do. REPOSITORY R265 KConfigWidgets REVISION

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

abetts added a comment. My opinion on this patch from the beginning has been that it really doesn't add much more than was there before. Visually, it clutters the UI. The icon selected for it might also not be visually appealing or meaningful enough. I believe there should be a different

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ngraham added a comment. Yeah, I'm sorry about that. If VDG people ask for something that's technically impossible, you've gotta push back on that. They often don't know what is and isn't possible, or reasonable. We've been trying to help VDG people be more technical so they don't

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin added a comment. You know it started as a proper painted indicator within the widget area, right? As such it couldn't have any of the issues you're pointing out now... Who pushed me to have them at distance I wonder? Right, was people from the VDG. So I find grand that then it goes

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ngraham added a subscriber: GB_2. ngraham added a comment. In D27540#652692 , @ervin wrote: > In D27540#652664 , @ngraham wrote: > > > David asked for VDG to approve before this landed, which wasn't

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin added a comment. In D27540#652669 , @ngraham wrote: > Finally clicking on the revert button in Spectacle's settings page causes a segfault for me: P590 Spectacle crash backtrace Couldn't quite

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin added a comment. In D27540#652664 , @ngraham wrote: > David asked for VDG to approve before this landed, which wasn't done. Dude, I jumped through all the hoops for the past weeks. Also it got no further reply after I updated the

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ngraham added a comment. I have concerns about this. The button has no tooltip so it's not obvious what will happen when clicked on. It should at least say "Revert to default setting" in a tooltip, and preferably even the name or text of the default setting that will be reverted to.

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ngraham added a comment. David asked for VDG to approve before this landed, which wasn't done. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis, broulik Cc: alexde, ndavis, iasensio,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

This revision was automatically updated to reflect the committed changes. Closed by commit R265:11186c056198: KCModule: Indicate when a setting has been changed from the default or previous… (authored by ervin). REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

davidedmundson accepted this revision. davidedmundson added a comment. This revision is now accepted and ready to land. Assuming VDG are happy, go for it. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: ervin, ngraham, davidedmundson, meven, crossi,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin updated this revision to Diff 79656. ervin marked 6 inline comments as done. ervin added a comment. Addresses David's comments REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27540?vs=78703=79656 REVISION DETAIL

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

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

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin added a comment. In D27540#638985 , @ndavis wrote: > Somehow I missed the notification that this was updated. > > Thanks for the horizontal alignment. Could you also add a left margin to the column of reset buttons? It should be the

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin edited the test plan for this revision. 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,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ndavis added a comment. Somehow I missed the notification that this was updated. Thanks for the horizontal alignment. Could you also add a left margin to the column of reset buttons? It should be the same as the spacing between the labels and the controls, which is

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

davidedmundson added a comment. What do we want to happen for released code that gets a bugfix update? INLINE COMMENTS > kconfigdialogmanager.cpp:609 > +const auto defaultValue = [item] { > +item->swapDefault(); > +const auto value = item->property(); Why not

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin updated this revision to Diff 78703. ervin added a comment. As advised by Kai and David on D27840 , switch to using tool buttons and fix RTL handling. REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin added a reviewer: broulik. 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,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin added a comment. In D27540#629380 , @ervin wrote: > In D27540#629362 , @ndavis wrote: > > > Is it possible to align all of the reset buttons like a column? > > > Why I'm not surprised.

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin edited the test plan for this revision. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin updated this revision to Diff 78468. ervin added a comment. Have the indicators vertically line up automatically REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27540?vs=77847=78468 REVISION DETAIL https://phabricator.kde.org/D27540

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin added a comment. In D27540#629362 , @ndavis wrote: > Is it possible to align all of the reset buttons like a column? Why I'm not surprised. ;-) Honestly with enough code it might be possible, but that'd be expensive in term of

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ndavis added a comment. Is it possible to align all of the reset buttons like a column? REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis Cc: alexde, ndavis, iasensio, davidre,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin edited the test plan for this revision. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin edited the summary of this revision. ervin edited the test plan for this revision. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis Cc: alexde, ndavis, iasensio, davidre,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin updated this revision to Diff 77847. ervin added a comment. Take feedback about the GUI into account REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27540?vs=76362=77847 REVISION DETAIL https://phabricator.kde.org/D27540 AFFECTED FILES

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin added a comment. In D27540#627618 , @ndavis wrote: > Some extra rules I thought of: > > - With the checkable label example in the mockup above, it should reset both the label and the checkbox. Just for the record, this will

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ndavis added a comment. In D27540#627615 , @ngraham wrote: > @ndavis I know you had some idea for which icon to use, and an idea to make it a clickable button, right? Right, it's like MuseScore: F8176442:

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ngraham added a reviewer: ndavis. ngraham added a comment. @ndavis I know you had some idea for which icon to use, and an idea to make it a clickable button, right? REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: ervin, ngraham, davidedmundson,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin added a comment. In D27540#617485 , @ervin wrote: > And now you got a screenshot as well. Waiting for further feedback now. Ping? This patch was first created three weeks ago now. Note that look in D27840

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ervin added a comment. In D27540#617589 , @alexde wrote: > Does this patch also fix indirectly #274629? I doubt it, this bug report seems confused BTW... it was never broken on the kdelibs side but *some* dialogs have bugs which break

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

alexde added a comment. Does this patch also fix indirectly #274629? REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n,

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ndavis added a comment. I can see the utility in indicating non-default values in some cases, particularly with software that has a lot of necessary complexity in the settings or where non-default values can cause problems (see SVG Cleaner GUI for an example of both). However, I don't think

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

ngraham retitled this revision from "KCModule: Indicate when a setting has been changed from the default value" to "KCModule: Indicate when a setting has been changed from the default or previous value". REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D27540 To: