Move the inline preview button into the menu
AbandonedPublic

Authored by ngraham on May 1 2018, 8:45 PM.

Details

Reviewers
rkflx
Group Reviewers
Frameworks
Maniphest Tasks
T8552: Polish Open/Save dialogs
Summary

Once previews are on by default and automatically disable themselves for small icon sizes, there's little reason to have the button to toggle them presented to centrally.

This patch moves it to the menu and removes its icon to prevent it from being the only item in that menu that gets an icon.

Test Plan

Diff Detail

Repository
R241 KIO
Branch
inline-preview-button-in-menu (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
ngraham created this revision.May 1 2018, 8:45 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMay 1 2018, 8:45 PM
ngraham requested review of this revision.May 1 2018, 8:45 PM
rkflx added a comment.May 2 2018, 10:30 AM
In T8552#140332, @rkflx wrote:

moving the Show Preview button to the menu

Makes sense to me, but might be controversial with others.

FWIW, if people do not like this (e.g. because they often toggle previews, or because it should look consistent with Dolphin), I'd also be fine with keeping the button if in the end it turns out we have the space for it.

Keeping the button would also allow to show the tooltip text, explaining why the action sometimes is disabled. Having no such explanation for the menu item feels a bit awkward.

Patch itself LGTM, but needs more +1's on the idea.

Now that previews are on by default but not shown for small icons, are there any other objections to doing this?

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald TranscriptMay 10 2018, 1:11 PM

Friendly ping!

Should we land this on the file-dialog-improvements branch, or are folks still unsure?

rkflx resigned from this revision.Aug 24 2018, 10:46 PM
ngraham abandoned this revision.Mar 19 2019, 11:44 AM

I don't think this makes sense anymore.

ngraham edited the summary of this revision. (Show Details)Mar 19 2019, 11:45 AM
meven added a subscriber: meven.Apr 25 2019, 7:49 PM

I don't think this makes sense anymore.

"Put the Show Inline Preview button in the settings menu" is still in T8552, shouldn't we remove this there then ?

I don't think this makes sense anymore.

"Put the Show Inline Preview button in the settings menu" is still in T8552, shouldn't we remove this there then ?

@ngraham ?

Oops, sorry I missed your comments! Yes, removing it from the task description makes sense.