D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

This revision was automatically updated to reflect the committed changes. Closed by commit R119:943140d368f5: Allow 'Exclude folders' section in Baloo KCM to fill window (authored by kishoreg, committed by ngraham). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D21414?vs=58849&id=58855#to

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Thanks so much. I'll land this on the 5.16 branch. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21414 To: kishoreg, #baloo, #vdg, ngraham Cc: GB_2, abe

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg updated this revision to Diff 58849. kishoreg edited the test plan for this revision. kishoreg added a comment. Removed colon to keep string unchanged. Screenshot: F6858284: Screenshot_20190529_201418.png REPOSITORY R119 Plasma Desktop C

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

ngraham added a comment. If the initial size is fine, that's good enough. We don't need to prevent people from resizing the window to be really small. If you remove the colon from the string (see earlier discussion), I think this could land in 5.16! REPOSITORY R119 Plasma Desktop REV

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg added a comment. In D21414#471615 , @ngraham wrote: > With no items in the list view, the size looks perfect to me: > > F6858199: Screenshot_20190529_080727.png > > Are you seeing something

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

ngraham added a comment. With no items in the list view, the size looks perfect to me: F6858199: Screenshot_20190529_080727.png Are you seeing something different? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg updated this revision to Diff 58838. kishoreg edited the test plan for this revision. kishoreg added a comment. Moved buttons to the left. F6858110: Screenshot_20190529_180905.png I'll try to figure out how to set the size hint properly. RE

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

ngraham added a comment. In D21414#471289 , @kishoreg wrote: > BTW, if I open the window standalone (from krunner), it allows me to resize it vertically so that the entire window gets scrollbars. F6856837: Screenshot_20190528_220702.png

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

ngraham added a comment. In D21414#471286 , @GB_2 wrote: > In D21414#471285 , @ngraham wrote: > > > Actually now that I think about it, this should probably go into Plasma 5.16. However since I aske

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg added a comment. BTW, if I open the window standalone (from krunner), it allows me to resize it vertically so that the entire window gets scrollbars. F6856837: Screenshot_20190528_220702.png I can't do this horizontally. Is there some vertical

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

GB_2 added a comment. In D21414#471285 , @ngraham wrote: > Actually now that I think about it, this should probably go into Plasma 5.16. However since I asked you to add a colon to the end of a string, technically that's a string change and thos

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

ngraham added a comment. Actually now that I think about it, this should probably go into Plasma 5.16. However since I asked you to add a colon to the end of a string, technically that's a string change and those aren't allowed so close to the release, so it would have to go into Plasma 5.17

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

ngraham added a comment. Nope, that looks perfect to me! REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21414 To: kishoreg, #baloo, #vdg, ngraham Cc: abetts, bruns, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jen

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg updated this revision to Diff 58797. kishoreg added a comment. Screenshot as of this diff: F6856823: Screenshot_20190528_220210.png Are there any other changes that need to be made? REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDAT

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

abetts added a comment. +1 REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21414 To: kishoreg, #baloo, #vdg, ngraham Cc: abetts, bruns, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

ngraham added a comment. I think I confused you, sorry. My desire was for some spacing above the listview's label, like this: F6856571: edited.png REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21414 To: kishoreg,

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg updated this revision to Diff 58786. kishoreg added a comment. This is awkward. I hadn't realized that I was supposed to use one space for indentation. Fixed as per the preexisting convention. REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg updated this revision to Diff 58784. kishoreg added a comment. I seem to have messed up the indentation in my previous diff. Here it is with the indentation fixed. REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21414?vs=58781&id=58784 REVI

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg marked an inline comment as done. kishoreg added a comment. BTW, checkboxes in other KCMs also seem to have the same spacing as this one had before this change. Is there a way to globally apply this spacing / padding consistently to QCheckBoxes in all KCMs for the sake of consistency

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg updated this revision to Diff 58781. kishoreg added a comment. I've added a colon after the 'Do not search in these locations' text. I've also added spacing between the checkboxes and their text. I've put in 10 px for now. Is there a better way to do this? Without additional spa

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

ngraham added a comment. Great! I think a bit of padding between the checkbox and the label would help too. Then this'll be a great UI upgrade! INLINE COMMENTS > configwidget.ui:60 > + > + Do not search in these locations > + Now this string needs to end wi

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg updated this revision to Diff 58723. kishoreg edited the summary of this revision. kishoreg added a comment. This patch incorporates Nate's suggestion to remove the box around the 'exclude folders' area. I've added screenshots of all three cases (with spacer, with box, and without bo

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg added a comment. Before change: F6854280: Screenshot_20190519_105621.png Initial patch: F6854282: Screenshot_20190527_181359.png With box removed: F6854285: Screenshot_20190527_190128.png

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

ngraham added a comment. Tested; LGTM. While you're cleaning up the UI, you could probably remove the group box too, and just add a left-aligned label above the list view. What do you think? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21414 To: kishoreg,

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

bruns added a comment. The code change looks sane for me, although I haven't tested it. The summary should mention: - what is currently wrong (space consumed by spacer) - what was done to fix it (spacer removed, so the folder list can stretch) Can you add a before/after screens

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

kishoreg created this revision. kishoreg added a reviewer: Baloo. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. kishoreg requested review of this revision. REVISION SUMMARY I just removed a spacer. It seems like it was added intentionally, so I'm not sure if removing