Add QuickLook support #2082

VictoriousRaptor · 2023-04-22T09:43:16Z

Refined version of #859. Too many merging conflict so I just create a new branch instead. Compatible with our preview function.

Changes

Add a property FilePath for Result.Preview to indicate what file should QuickLook preview.
- Explorer Plugin support
An option to choose using QuickLook or not. Default is false.

Behavior

Preview Hotkey is used to toggle QuickLook in Flow.
If built-in preview is already open, QuickLook won't be triggered before user closes built-in preview. It's to deal with "Always Preview" option and switching between results that supports QuickLook or not.
When a result has non-null Preview.FilePath try to use QuickLook to preview.
Flow's preview and QuickLook don't open at the same time. If a result has non-null Preview.FilePath pressing hotkey only triggers QuickLook.
When QuickLook is unavailable, a notification is sent to tell users to check status.
QL is forcibly closed when hiding flow.
If Flow "thinks" QL is open, a lost focus won't close Flow. This is to make sure users can interact with QL's UI controls. But when QL is closed by clicking its close button, opening QL again and interact with it will close Flow.

Problems

~~Should "Always Preview" open QuickLook?~~ No
~~When "Always Preview" is on (which means internal preview is on), selecting another result that can use QuickLook triggers QuickLook. Looks bad.~~
~~When "Use QuickLook" is on but QuickLook is not launched or installed what kind of warning should be send to users?~~ Send a notification
~~No QuickLook API to set on top. (Open selected result in quicklook #859 (comment))~~ Manually set it.
~~Switching from external preview result to builtin result doesn't properly trigger builtin preview.~~

TODO

Roughly tested but need some more tests and code review. Generally looking good so far.
A better glyph in settings.

Screenshot

- Adapted from Files

- Internal and external preview can't open at the same time - Always preview option doesn't control external

Garulf · 2023-04-22T18:07:49Z

Just got a chance to test run this. It looks very good! I have a few opinions on some of your problems listed:

Should "Always Preview" open QuickLook?

I don't think this works well with quick look as both windows overlap.

When "Use QuickLook" is on but QuickLook is not launched or installed what kind of warning should be send to users?

I think we can resolve this by greying out this option if a QuickLook installation is not detected. This can be tricky though as QuickLook is available in many different forms including the Windows app store.

Instead of tracking all this down we could send a pipe message and check its status to see if quick look is available and determine to enable the option.

No QuickLook API to set on top. (#859 (comment))

This was also an issue with the original PR.

jjw24 · 2023-04-22T22:54:52Z

QuickLook functionality should be provided fully by a plugin. To achieve this we need to use this #1013, and this pr is a working POC already.

Garulf · 2023-04-23T00:06:23Z

QuickLook functionality should be provided fully by a plugin. To achieve this we need to use this #1013, and this pr is a working POC already.

I don't think @VictoriousRaptor was around for that. But yes as a plugin would be ideal.

Also I would use this PR in favor of my older code. This is more fleshed out.

jjw24 · 2023-04-23T00:23:04Z

Maybe best that @VictoriousRaptor just continue work as is with this one so we can keep the momentum, once I helped onesound I will come back to finishing off that pr 1013 and migrate the finished code here into a plugin.

VictoriousRaptor · 2023-04-23T03:00:29Z

Maybe best that @VictoriousRaptor just continue work as is with this one so we can keep the momentum, once I helped onesound I will come back to finishing off that pr 1013 and migrate the finished code here into a plugin.

Ok I'll do some more tests on this PR and try to figure out the problems.

VictoriousRaptor · 2023-04-23T07:41:54Z

When "Use QuickLook" is on but QuickLook is not launched or installed what kind of warning should be send to users?

I think we can resolve this by greying out this option if a QuickLook installation is not detected. This can be tricky though as QuickLook is available in many different forms including the Windows app store.

Instead of tracking all this down we could send a pipe message and check its status to see if quick look is available and determine to enable the option.

Greying out option is good. And what if user enables this option (with QuickLook installed and running), and then quit QuickLook and preview in Flow? Should we fallback to Flow's preview? And what kind of message to tell users we are not able to launch QuickLook? Maybe a notification toast?

No QuickLook API to set on top. (#859 (comment))

This was also an issue with the original PR.

Not a big deal. Just manually set it once and for all.

jjw24 · 2024-05-26T00:30:38Z

Can't this be detected from user settings, preview -> built-in or QuickLook

VictoriousRaptor · 2024-05-26T01:37:56Z

Can't this be detected from user settings, preview -> built-in or QuickLook

This is the current implementation but logic to deciding to use which preview is not clear. I spent quite a lot of time on it.

I don't have time to review this PR this week so if we want to merge now it requires more detailed review from the team.

jjw24 · 2024-05-29T04:10:06Z

Updated this PR with (see commits):

support plugin with external preview via functionality interface.
enable/disable external preview by toggling plugin on/off
updated preview visibility logic.
added a QuickLook plugin for testing, but will be a published standalone plugin and not merging with this PR.

VictoriousRaptor · 2024-05-30T13:28:34Z

Flow.Launcher/MainWindow.xaml.cs

@@ -622,7 +622,7 @@ private async void OnDeactivated(object sender, EventArgs e)
                if (_settings.UseAnimation)
                    await Task.Delay(100);

-                if (_settings.HideWhenDeactivated)
+                if (_settings.HideWhenDeactivated && !_viewModel.ExternalPreviewVisible)


This is for when you click away from the mouse it doesnt hide both the search window and preview. Not sure which is user-friendlier, hide when deactivated or do not.

VictoriousRaptor · 2024-05-30T13:46:16Z

Flow.Launcher/ViewModel/MainViewModel.cs

+                    break;
+
+                case true
+                    when !ExternalPreviewVisible && Settings.AlwaysPreview && CanExternalPreviewSelectedResult(out var path):


When I enable AlwaysPreview QuickLook automatically pops whenever I type. Not usable imo.

Yeah i know, there is the option to switch AlwaysPreview off. The real problem is QuickLook doesnt remember last position, but nothing we can do.

So AlwaysPreview shouldn't control external preview.

I want to align the behaviour for both internal and external so logic is a bit more straightforward, and also not just build logic for QuickLook because in the future we may integrate with another preview tool that works well with always preview option. For now user can just switch it off.

Is it possible to set a "always external preview" in preview plugin?

Hmm, let's not not add additional logic when there is already a button that does exactly that.

Flow.Launcher/ViewModel/MainViewModel.cs

VictoriousRaptor · 2024-06-09T17:56:59Z

Flow.Launcher/ViewModel/MainViewModel.cs

@@ -871,7 +871,7 @@ public void ResetPreview()
        {
            if (Settings.AlwaysPreview)
            {
-                ShowPreview();
+                ShowInternalPreview();


When AlwaysPreview is true, specify internal preview.

This is because QL is not suitable for using with always preview right?

This is because QL is not suitable for using with always preview right?

Yep

What happens when we have other preview tools that is suitable?

What happens when we have other preview tools that is suitable?

Well maybe pass always preview to plugin and let plugin decide? If plugin returns false then use internal preview.

Done. Let me know if good to merge.

VictoriousRaptor · 2024-06-09T17:58:59Z

Plugins/Flow.Launcher.Plugin.QuickLook/Flow.Launcher.Plugin.QuickLook.csproj

@@ -0,0 +1,51 @@
+<Project Sdk="Microsoft.NET.Sdk">


If we merge is this plugin going to be a default one?

No I will take it out before merge. There is already a QL plugin repo in the org.

VictoriousRaptor added 17 commits April 21, 2023 22:13

Add FilePath property for PreviewInfo

71376b8

Add FilePath for Explorer results preview

96f5231

Default values for preview info

0c4f50a

Add file path in preview

1cedb6e

Move QuickLook support to QuickLookHelper

f460913

- Adapted from Files

Add open/close QuickLook function

0b6f737

Reorganize preview code

ab20dcc

Implement open QuickLook feature

98991fe

Fix case when switching file

99b5197

Add UI to toggle QuickLook usage

1e1f29b

Tweak Win32 preview path

afa8a34

Implement preview switching logic

a1600fa

- Internal and external preview can't open at the same time - Always preview option doesn't control external

Rename method for clarity

cf10a4a

Reduce unnecessary image load

10d1410

Fix switching preview logic

ba8bb3a

Hide external preview when hiding flow

098e90b

comment

26ff98e

VictoriousRaptor requested a review from Garulf April 22, 2023 09:43

This comment has been minimized.

Sign in to view

Garulf mentioned this pull request Apr 22, 2023

Open selected result in quicklook #859

Closed

3 tasks

jjw24 assigned VictoriousRaptor Apr 22, 2023

jjw24 added the enhancement New feature or request label Apr 22, 2023

jjw24 added this to the Future milestone Apr 22, 2023

Formatting

9061da9

jjw24 added the review in progress Indicates that a review is in progress for this PR label May 26, 2024

jjw24 enabled auto-merge (squash) May 26, 2024 00:28

jjw24 added 5 commits May 28, 2024 21:27

add plugin support for external preview

666211d

update external preview methods to use the external preview interface

8da1313

updated PreviewVisible logic to base on ResultAreaColumn size

587536f

updated internal and external preview's show and hide logic

53e4bbb

Merge branch 'external_preview_plugin_support' into quicklook

b345944

jjw24 mentioned this pull request May 28, 2024

Allow plugins to capture hotkey actions on query window results #1013

Closed

add QuickLook plugin for testing

70e530b

remove QuickLook related code from flow

493a30e

jjw24 force-pushed the quicklook branch from 6f5cfcb to 493a30e Compare May 29, 2024 06:12

jjw24 added 2 commits May 29, 2024 21:17

Merge remote-tracking branch 'origin/dev' into quicklook

f902eae

minor clean up and update

9e39e48

VictoriousRaptor commented May 30, 2024

View reviewed changes

Flow.Launcher/ViewModel/MainViewModel.cs Show resolved Hide resolved

VictoriousRaptor modified the milestones: Future, 1.19.0 May 30, 2024

VictoriousRaptor disabled auto-merge May 30, 2024 13:51

jjw24 and others added 4 commits May 31, 2024 17:02

minor clean up

f4f519b

Merge branch 'dev' into quicklook

5ec474c

allow internal preview when external not available

2e47da4

Don't show external preview when always preview enabled

e6f0f28

VictoriousRaptor commented Jun 9, 2024

View reviewed changes

jjw24 added 2 commits June 12, 2024 14:14

allow preview plugin to override the AlwaysPreview setting

4dbecb1

Merge remote-tracking branch 'origin/dev' into quicklook

03686dd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add QuickLook support #2082

Add QuickLook support #2082

VictoriousRaptor commented Apr 22, 2023 •

edited by jjw24

This comment has been minimized.

Garulf commented Apr 22, 2023

jjw24 commented Apr 22, 2023

Garulf commented Apr 23, 2023

jjw24 commented Apr 23, 2023

VictoriousRaptor commented Apr 23, 2023

VictoriousRaptor commented Apr 23, 2023

jjw24 commented May 26, 2024

VictoriousRaptor commented May 26, 2024

jjw24 commented May 29, 2024 •

edited

VictoriousRaptor May 30, 2024

jjw24 May 31, 2024

VictoriousRaptor May 30, 2024

jjw24 May 31, 2024

VictoriousRaptor May 31, 2024

jjw24 Jun 3, 2024 •

edited

VictoriousRaptor Jun 3, 2024

jjw24 Jun 3, 2024

VictoriousRaptor Jun 9, 2024

jjw24 Jun 10, 2024

VictoriousRaptor Jun 10, 2024

jjw24 Jun 10, 2024

VictoriousRaptor Jun 11, 2024

jjw24 Jun 12, 2024

VictoriousRaptor Jun 9, 2024

jjw24 Jun 10, 2024

Add QuickLook support #2082

Are you sure you want to change the base?

Add QuickLook support #2082

Conversation

VictoriousRaptor commented Apr 22, 2023 • edited by jjw24

Changes

Behavior

Problems

TODO

Screenshot

This comment has been minimized.

Garulf commented Apr 22, 2023

jjw24 commented Apr 22, 2023

Garulf commented Apr 23, 2023

jjw24 commented Apr 23, 2023

VictoriousRaptor commented Apr 23, 2023

VictoriousRaptor commented Apr 23, 2023

jjw24 commented May 26, 2024

VictoriousRaptor commented May 26, 2024

jjw24 commented May 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjw24 Jun 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VictoriousRaptor commented Apr 22, 2023 •

edited by jjw24

jjw24 commented May 29, 2024 •

edited

jjw24 Jun 3, 2024 •

edited