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

Command palette search results should bold the matching text #6646

Closed
zadjii-msft opened this issue Jun 23, 2020 · 13 comments
Closed

Command palette search results should bold the matching text #6646

zadjii-msft opened this issue Jun 23, 2020 · 13 comments
Assignees
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@zadjii-msft
Copy link
Member

Omitted from initial implementation in #6635.

I'm really unsure if this is possible in XAML, but hey, I think it would be important.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jun 23, 2020
@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Jun 23, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jun 23, 2020
@zadjii-msft zadjii-msft added the Help Wanted We encourage anyone to jump in on these. label Jun 23, 2020
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Jun 23, 2020
@DHowett
Copy link
Member

DHowett commented Jun 23, 2020

If we're using TextBlocks we can just fill them with Runs generated at runtime 😄

@DHowett
Copy link
Member

DHowett commented Jun 23, 2020

image

<TextBlock FontFamily="Segoe UI" FontSize="32">
  <Run FontWeight="Bold">N</Run><Run>ew</Run> <Run FontWeight="Bold">T</Run><Run>ab</Run>
</TextBlock>

@zadjii-msft
Copy link
Member Author

Yes, that's easy enough, but then we need to hook up a lot more XAML - the Palette knows which characters need to be highlighted, so it would need to set that on each of the commands in the match list, then each of those guys would need to return a Run instead of just a string.

It's not impossible, but it's a lot more XAML binding than anything I've done

@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 25, 2020
@DHowett
Copy link
Member

DHowett commented Jun 25, 2020

Mind if I bump this one into 2.0? If you don't get to it, I'd like to. 😄

@zadjii-msft
Copy link
Member Author

Sure thing, it's done

@DHowett
Copy link
Member

DHowett commented Jun 25, 2020

Thanks!

@Don-Vito
Copy link
Contributor

@DHowett, @zadjii-msft - so this was planned to be a nice warmup exercise for Saturday morning. This was a bad decision 😄.. aka how to ruin a half of your weekend..

  • Defining a model that instead of a name stores a vector of matched and not matched segments - easy
  • Now let's bind it to the runs - the WinRT Textblock::Inlines are not bindable (ouch)
  • No problem, let's register an attached property and create runs in code behind - coded everything (even with nice converter), XAML complains on the binding. Reading WinRT docs (ouch 2):
    image
  • Ha, good luck defining imperatively dynamic context without tons of code-behind. No problem, I will simply create a custom control. WinRT TextBlock is sealed (ouch 3)
  • Found myself writing a User Control 😞

On a positive side, it work.. on a negative I need to do a lot of cleanups before this worth a PR
SearchHighlightNoMouse

@DHowett
Copy link
Member

DHowett commented Oct 19, 2020

Hey! I worked on this for a bit too, actually, during a hackathon! I ended up binding the content of a Border to a property that produces a TextBlock itself with its own inline runs constructed at runtime.

I’ve been meaning to put it up for review, but I can hold off so we don’t have conflicting changes.

Thanks for working on this!

@Don-Vito
Copy link
Contributor

@DHowett - I kinda enjoyed 😄 . In any case please let me know if you believe it is something I should continue to productize - since if you have a working solution than probably we can skip mine.

BTW, I am not complaining and not regretting anything, but consider removing "help wanted" - I've seen some additional items, that were marked as "help wanted" but were at different stages of implementation.

@DHowett
Copy link
Member

DHowett commented Oct 19, 2020

That’s a very good call (about not keeping the help tag on things we start working on). I expect your solution is better than mine—after all, mine was little more than a hack—so I’d love to see you finish it. A label that can highlight specific ranges is a very useful general-purpose UserControl.

@Don-Vito
Copy link
Contributor

@DHowett - from what I've learnt observing this community is that your "hacks" are much better than the best contribution I can do! 😄 In any case will continue to productize it during this week (it is in a bad shape, as I didn't work with neither C++, nor XAML in the last 8 years).

@zadjii-msft
Copy link
Member Author

@Don-Vito I love it! Looking forward to the PR ☺️

@zadjii-msft zadjii-msft removed the Help Wanted We encourage anyone to jump in on these. label Oct 19, 2020
@ghost ghost added the In-PR This issue has a related PR label Oct 20, 2020
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Nov 6, 2020
@DHowett DHowett closed this as completed in 1aff3bc Nov 6, 2020
@ghost
Copy link

ghost commented Nov 11, 2020

🎉This issue was addressed in #7977, which has now been successfully released as Windows Terminal Preview v1.5.3142.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

No branches or pull requests

3 participants