D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12986 To: ngraham, #dolphin, #frameworks, #vdg, davidedmundson Cc: davidedmundson, abetts, rkflx, kfm-devel, kde-frameworks-devel, michaelh, ngraham, bruns

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added a comment. Got it, need to remember about the string freeze... REPOSITORY R241 KIO BRANCH clean-up-trash-kcm (branched from master) REVISION DETAIL https://phabricator.kde.org/D12986 To: ngraham, #dolphin, #frameworks, #vdg, davidedmundson Cc: davidedmundson, abetts,

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

davidedmundson added a comment. Please wait till the 3rd. Otherwise you give practically no days for the i18n changes to happen. But then yes go for it. For future if I give an acceptance with a really minor comment feel free to push straight away after. REPOSITORY R241 KIO

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added a comment. Friendly ping! If nobody objects, I will commit this on June 3rd, or sooner if I get more explicit approvals. REPOSITORY R241 KIO BRANCH clean-up-trash-kcm (branched from master) REVISION DETAIL https://phabricator.kde.org/D12986 To: ngraham, #dolphin,

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham marked 2 inline comments as done. REPOSITORY R241 KIO BRANCH clean-up-trash-kcm (branched from master) REVISION DETAIL https://phabricator.kde.org/D12986 To: ngraham, #dolphin, #frameworks, #vdg, davidedmundson Cc: davidedmundson, abetts, rkflx, kfm-devel, kde-frameworks-devel,

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham updated this revision to Diff 34818. ngraham added a comment. Don't need to set a parent for `mLimitReachedAction`, since the `QFormLayout` it's in will just re-parent it anyway REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12986?vs=34815=34818

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added inline comments. INLINE COMMENTS > davidedmundson wrote in kcmtrash.cpp:303 > There's no benefit in changing the initial parent of these. > > It's all moot as QFormLayout::addItem takes ownership Ah right, I think I wrote this before I found that out. In fact, you can omit the

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

davidedmundson accepted this revision. davidedmundson added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > kcmtrash.cpp:303 > > -mLimitReachedAction = new QComboBox(mSizeWidget); > -mLimitReachedAction->addItem(i18n("Warn Me")); > +

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added a comment. This revision improves the labels per review comments, but stays with a combobox instead of radio buttons in the interest of avoiding too many code changes in this patch, since the glue code changes quite a bit when you switch controls. If folks feel strongly that

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12986 To: ngraham, #dolphin, #frameworks, #vdg Cc: abetts, rkflx, kfm-devel, kde-frameworks-devel, michaelh, ngraham, bruns

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham updated this revision to Diff 34815. ngraham added a comment. Tweaks per the review process REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12986?vs=34488=34815 BRANCH clean-up-trash-kcm (branched from master) REVISION DETAIL

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added a comment. In D12986#265699 , @abetts wrote: > What should be the end result of this patch? Hopefully something like our latest mockup: F5860717: new version.png Do folks

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

abetts added a comment. In D12986#265687 , @rkflx wrote: > In my personal experience spending more time on a limited set of items leads to better results and less churn (as in reverting commits, number of iterations per patch etc.) than only

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. In my personal experience spending more time on a limited set of items leads to better results and less churn (as in reverting commits, number of iterations per patch etc.) than only lightly touching a large amount of issues everywhere. Of course you can do whatever

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

abetts added a comment. In D12986#265675 , @rkflx wrote: > In D12986#265666 , @ngraham wrote: > > > In D12986#265662 , @rkflx wrote: > > > > > Anyway,

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added a comment. What are you actually asking me to do? Stop changing strings? Stop advocating for design changes? Stop submitting patches that implement design changes? Submit better patches that don't require so much reviewer time? Stop submitting patches entirely? Do less of

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. > The value is in standardizing on something. Personally I'd be okay with a consistent style per application, with the new style applied to new applications, and allowing exceptions where it makes sense. You've already witnessed the amount of bikeshedding in

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. In D12986#265666 , @ngraham wrote: > In D12986#265662 , @rkflx wrote: > > > Anyway, I'm not complaining too much (but please be aware that every change you propose creates

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added a comment. In D12986#265662 , @rkflx wrote: > > The value is in standardizing on something. > > Personally I'd be okay with a consistent style per application, with the new style applied to new applications, and allowing

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added a comment. In D12986#265633 , @rkflx wrote: > Just noticed that I based my suggestion on your screenshot, but missed to see that you changed the semantics compared to the original dialog: The trash won't be emptied completely

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. Thanks, "Size" and "Full trash" are looking good now. In D12986#265235 , @rkflx wrote: > In D12986#265231 , @ngraham wrote: > > > Yeah, that makes sense. Suggestions

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added a comment. In D12986#265556 , @abetts wrote: > That looks really good IMHO. Thanks! A lot of the credit goes to Henrik, with his excellent eye for UI design. > Somewhat unrelated, I have seen a few UIs recently that

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

abetts added a comment. In D12986#265552 , @ngraham wrote: > Like this? > > F5860366: Radiobutton version.png That looks really good IMHO. Somewhat unrelated, I have seen a few UIs recently that

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added a comment. Like this? F5860366: Radiobutton version.png REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12986 To: ngraham, #dolphin, #frameworks, #vdg Cc: abetts, rkflx, kfm-devel, kde-frameworks-devel, michaelh,

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

abetts added a comment. In D12986#265235 , @rkflx wrote: > In D12986#265231 , @ngraham wrote: > > > Yeah, that makes sense. Suggestions welcome, of course. Is this any better? > > F5859259:

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added a comment. Good idea, perhaps some radio buttons would make more sense here. I'll play around with it and try other languages. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12986 To: ngraham, #dolphin, #frameworks, #vdg Cc: abetts, rkflx, kfm-devel,

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. In D12986#265231 , @ngraham wrote: > Yeah, that makes sense. Suggestions welcome, of course. Is this any better? > F5859259: After 2.png Hm, I think after the checkbox there

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

abetts added a comment. In D12986#265231 , @ngraham wrote: > Yeah, that makes sense. Suggestions welcome, of course. Is this any better? > F5859259: After 2.png > > Still not super happy with "When

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham added a comment. Yeah, that makes sense. Suggestions welcome, of course. Is this any better? F5859259: After 2.png Still not super happy with "When limit reached: REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12986

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

abetts added a comment. In D12986#265198 , @rkflx wrote: > I don't think putting whole phrases as a label before the checkbox works very well, and I would be surprised if that's what the HIG now recommends. I assume at most a single subject

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. I don't think putting whole phrases as a label before the checkbox works very well, and I would be surprised if that's what the HIG now recommends. I assume at most a single subject should be used there? For example, instead of the awkwardly placed checkbox in

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham edited the test plan for this revision. ngraham added a subscriber: kfm-devel. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12986 To: ngraham, #dolphin, #frameworks, #vdg Cc: kfm-devel, kde-frameworks-devel, michaelh, ngraham, bruns

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

ngraham created this revision. ngraham added reviewers: Dolphin, Frameworks, VDG. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. ngraham requested review of this revision. REVISION SUMMARY Clean up and standardize the UI to