D12328: Enable preview by default in the filepicker dialog
Henrik Fehlauer
noreply at phabricator.kde.org
Wed Apr 25 18:52:12 UTC 2018
rkflx requested changes to this revision.
rkflx added a comment.
This revision now requires changes to proceed.
In D12328#252246 <https://phabricator.kde.org/D12328#252246>, @rkflx wrote:
> In D12328#250028 <https://phabricator.kde.org/D12328#250028>, @anemeth wrote:
>
> > Rebase on D12321 <https://phabricator.kde.org/D12321>
>
>
> While the dependency tree is now looking good, the rebase did not work out correctly. While it might look correct on your local branch, on Phabricator you can clearly see that in this Diff the changes from the parent revision are visible, which should not be there. Please make sure to always check what ends up on Phabricator, as that's what your reviewers are seeing.
>
> `arc diff` will upload the commit range between `HEAD` and the upstream tracking branch. Here the upstream branch apparently is `master`, while it should be your local branch where D12321 <https://phabricator.kde.org/D12321> is living (you have a separate branch per Diff as written in the wiki, right?). You may want to look at my advice in D12321#249646 <https://phabricator.kde.org/D12321#249646> again.
@anemeth Ping. I won't be able to help you with `kconf_update` or review the patch unless Phab shows me the proper patch!
INLINE COMMENTS
> kdiroperator.cpp:2165
> if (d->inlinePreviewState == Private::NotForced) {
> - d->showPreviews = configGroup.readEntry(QStringLiteral("Previews"), false);
> + d->showPreviews = configGroup.readEntry(QStringLiteral("Preview Thumbnails"), true);
> + d->showPreviewsEnabledBeforeZoom = d->showPreviews;
There's still some confusion in the wording wrt. `Show Preview`.
How about naming your setting `Show Inline Previews` (inspired by `_k_toggleInlinePreview()`)?
(The other option could be renamed to `Show Aside Preview` in a followup patch.)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D12328
To: anemeth, #frameworks, #vdg, rkflx
Cc: abetts, rkflx, ngraham, #frameworks, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180425/d197c446/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list