Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extensions should be able to populate 'TextToDisplay' Equivalent from Run #121

Closed
joadoumie opened this issue Oct 25, 2024 · 6 comments
Closed
Labels
Area-API Anything having to do with the actual interface the extensions use to communicate with the host

Comments

@joadoumie
Copy link
Collaborator

Right now in Run, plugins can include 'QueryTextDisplay' which allows for some pretty cool auto-complete functionality.

I'm thinking we should allow for something equivalent where either one of two things happen or both?:

  1. We automagically include the text to be filled in without extensions even thinking about it. Meaning that from the users perspective we routinely will just be giving suggestions that can be completed with tab.

  2. Extensions can do a similar flow to what they do today and provide the string to be displayed and handle what happens when the user tabs.

To be more clear, here is what I'm talking about:

Animation

@joadoumie joadoumie added Area-HostUX UX elements of the host CmdPal application Area-3p Extensions Area-Inbox Extensions labels Oct 25, 2024
@zadjii-msft zadjii-msft added Area-API Anything having to do with the actual interface the extensions use to communicate with the host and removed Area-HostUX UX elements of the host CmdPal application Area-3p Extensions Area-Inbox Extensions labels Oct 26, 2024
@zadjii-msft
Copy link
Owner

Oh interesting. That's pretty cool idea. I'll ideate on this on the drive home.

Cause like, in the PT Run case, I'm guessing the suggestion is based on the "selected" element in the list. That presents an interesting API design choice here.

My initial thought is that it would make sense for this to be a property on IListPage itself - the search box is owned by the page. That however would be very chatty to update. The host would call page.SelectionChanged(item) and then asynchronously the extension would raise PropChanged("TextToDisplay"), which then the host would call page.TextToDisplay to fetch the new text. Three x-proc calls, every time the selection changes.

If we put it as a property on the list item itself, that's a little easier. Then every time we select a new item, we just display the item.TextToSuggest. Each item can suggest something if it wants.

heck I'm just gonna commit that right away.

@zadjii-msft
Copy link
Owner

I suppose my last question is:

Is the value of TextToDisplay the whole string that we fill in the search box? Or just append?

So in the registry example: when the user has HKEY_CURRENT_USER\ in the search box, and has HKEY_CURRENT_USER\AppEvents selected, is the TextToDisplay "HKEY_CURRENT_USER\AppEvents"

Actually, IMO that's the only thing that makes sense. Cause then the user can type HKCU and the TextToDisplay is HKEY_CURRENT_USER

Okay I don't have any questions

@zadjii-msft zadjii-msft added Area-HostUX UX elements of the host CmdPal application and removed Area-HostUX UX elements of the host CmdPal application labels Oct 26, 2024
@zadjii
Copy link
Collaborator

zadjii commented Nov 7, 2024

Oh okay the PT Run stuff is weird.

Cause it displays ghost text of the TextToDisplay of the first result, then when you up/down arrow, the TextToDisplay actually gets added to the text box BUT doesn't filter it. But then if you backspace at that point, the text in the box gets committed.

I kinda think that's weird. I think as you up/down through the list, you should only see ghost text till you hit to "accept" it. But I can be overridden on that matter.

I think we can probably get away with Just the ghost text for v0.1, and come back for a v0.2

@joadoumie
Copy link
Collaborator Author

Agree that feels weird.

I think what you described is precisely what I'd want as a user.

I was thinking though: why don't we make the default behavior of list items to have a text to display sorta situation that just works? At least for v0.1 that sorta just fulfills the nice autocomplete type thing and extensions don't have to think about it. We could then have an override or something if for some reason an extension wants a different behavior? Or maybe even a Boolean only that an extension just says yes I want to allow auto complete or no that messes with my aesthetic.

While we build it out I'll play around with PowerToys plugins to see if there are any interesting use cases where the text to display actually isn't just the result text.

@zadjii-msft
Copy link
Owner

While sitting up with Luca last night, I think I realized why it's like this.

If we just have a second text box at the end, showing the "remainder" of the TextToSuggest, then something like HKCU -> HKEY_CURRENT_USER doesn't... really work at all. So, you have to modify the actual text box itself to display it. We'll have to be careful to track changes to the field caused by us showing previews, vs actual changes to the search caused by typing. We only really want to send UpdateSearch(query) if the user actually types a character.

zadjii-msft added a commit that referenced this issue Nov 12, 2024
As discussed in #121. 

Adds a new `IListItem.TextToSuggest` property. If an extension populates that, and the user hits right-arrow when the item is selected, we'll set the `SearchText` to that text. 

I didn't give it a UI treatment, because 
1. I'm not good at XAML
2. As noted in #121 (comment), there's complicated edge cases here. You have to track what's actually been typed, and then also have a ghost suggestion for the very first item (before the user types/up/downs at all). 

I didn't think it was valuable to implement that right now before we have A Real App. 

* targets #144 
  * which targets #142 
    * which targets #141 

and those guys are all merged as of #150
@zadjii-msft
Copy link
Owner

This merged in #145 and then also #150 (cause I botched that merge)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-API Anything having to do with the actual interface the extensions use to communicate with the host
Projects
None yet
Development

No branches or pull requests

3 participants