D12647: Move the inline preview button into the menu

[Nathaniel Graham]

ngraham added a comment.


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

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks
Cc: meven, kde-frameworks-devel, anemeth, LeGast00n, michaelh, ngraham, bruns

D12647: Move the inline preview button into the menu

[Méven Car]

meven added a comment.


  In D12647#456283 , @meven wrote:
  
  > In D12647#434327 , @ngraham 
wrote:
  >
  > > 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 ?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks
Cc: meven, kde-frameworks-devel, anemeth, michaelh, ngraham, bruns

D12647: Move the inline preview button into the menu

[Méven Car]

meven added a comment.


  In D12647#434327 , @ngraham wrote:
  
  > 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 ?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks
Cc: meven, kde-frameworks-devel, anemeth, michaelh, ngraham, bruns

D12647: Move the inline preview button into the menu

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks
Cc: kde-frameworks-devel, anemeth, michaelh, ngraham, bruns

D12647: Move the inline preview button into the menu

[Nathaniel Graham]

ngraham abandoned this revision.
ngraham added a comment.


  I don't think this makes sense anymore.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks
Cc: kde-frameworks-devel, anemeth, michaelh, ngraham, bruns

D12647: Move the inline preview button into the menu

[Henrik Fehlauer]

rkflx resigned from this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks
Cc: kde-frameworks-devel, anemeth, michaelh, ngraham, bruns

D12647: Move the inline preview button into the menu

[Nathaniel Graham]

ngraham added a comment.


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

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks, rkflx
Cc: kde-frameworks-devel, anemeth, michaelh, ngraham, bruns

D12647: Move the inline preview button into the menu

[Nathaniel Graham]

ngraham added a comment.


  Friendly ping!

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks, rkflx
Cc: kde-frameworks-devel, anemeth, michaelh, ngraham, bruns

D12647: Move the inline preview button into the menu

[Nathaniel Graham]

ngraham added a comment.
Restricted Application added a subscriber: kde-frameworks-devel.


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

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks, rkflx
Cc: kde-frameworks-devel, anemeth, michaelh, ngraham, bruns

D12647: Move the inline preview button into the menu

[Henrik Fehlauer]

rkflx added a comment.


  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.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks, rkflx
Cc: anemeth, michaelh, bruns

D12647: Move the inline preview button into the menu

[Nathaniel Graham]

ngraham added a task: T8552: Polish Open/Save dialogs.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks, rkflx
Cc: anemeth, michaelh, bruns

D12647: Move the inline preview button into the menu

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12647

To: ngraham, #frameworks, rkflx
Cc: anemeth, michaelh, bruns

D12647: Move the inline preview button into the menu

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Frameworks, rkflx.
Restricted Application added a project: Frameworks.
ngraham requested review of this revision.

REVISION 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.
  
  Depends on D12328 

TEST PLAN
  [image]

REPOSITORY
  R241 KIO

BRANCH
  inline-preview-button-in-menu (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D12647

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kfilewidget.cpp

To: ngraham, #frameworks, rkflx
Cc: anemeth, michaelh, bruns