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

Feature: Added a Command Palette #12977

Merged
merged 15 commits into from
Jul 28, 2023
Merged

Conversation

hishitetsu
Copy link
Member

@hishitetsu hishitetsu commented Jul 19, 2023

Resolved / Related Issues

  • Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
    Closes Feature: Add command palette #2780

Validation
How did you test these changes?

  • Did you build the app and test your changes?
  • Did you check for accessibility? You can use Accessibility Insights for this.
  • Did you remove any strings from the en-us resource file?
    • Did you search the solution to see if the string is still being used?
  • Did you implement any design changes to an existing feature?
    • Was this change approved?
  • Are there any other steps that were used to validate these changes?
    1. Type ">" in the address bar or press Ctrl+Shift+P to open the command palette
    2. You can search commands by both description (translated) and command code.
    3. Select a suggestion or type a command code to invoke the command.

Screenshots
image

@0x5bfa
Copy link
Member

0x5bfa commented Jul 19, 2023

How about this?

  • When user entered >, display custom glyph with background accent color filled.
    (made by @mdtauk)
    image

@hishitetsu
Copy link
Member Author

  • When user entered >, display custom glyph with background accent color filled.

Not implemented yet. In fact, I'm not sure how to implement it.

@yaira2 yaira2 changed the title Feature: Command palette Feature: Added Command Palette Jul 20, 2023
@mdtauk
Copy link
Contributor

mdtauk commented Jul 22, 2023

  • When user entered >, display custom glyph with background accent color filled.

Not implemented yet. In fact, I'm not sure how to implement it.

I would suggest perhaps looking at the Tokenizing TextBox from the Community Toolkit
image

Or just modifying the template to shift the Text part 32px in from the left, and overlay an icon?

@0x5bfa
Copy link
Member

0x5bfa commented Jul 22, 2023

Or just modifying the template to shift the Text part 32px in from the left, and overlay an icon?

I would rather do this.

  • If you press backspace, that indicator would disappear.
  • If you press > it would appear.

@yaira2 yaira2 requested a review from 0x5bfa July 25, 2023 15:12
Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiny things

@hishitetsu hishitetsu requested a review from 0x5bfa July 27, 2023 02:53
0x5bfa

This comment was marked as resolved.

Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in terms of codebase quality

@yaira2
Copy link
Member

yaira2 commented Jul 28, 2023

@hishitetsu I tested the feature and it's working really well, the only feedback I have is that we should remove the command name, and that we should hide commands that aren't enabled.

@hishitetsu
Copy link
Member Author

we should remove the command name

The command name is important for non-English users because they will want to search for commands both in their native language and in English. Would it be better to show the English description rather than the command name, which is what Visual Studio Code does?
image

we should hide commands that aren't enabled

I don't think so because users may want to know what commands exist. But you want to hide them, I will do so.

@yaira2
Copy link
Member

yaira2 commented Jul 28, 2023

@hishitetsu I meant only to hide the name, we can still include the name in the search query.

I don't think so because users may want to know what commands exist. But you want to hide them, I will do so.

I personally found them confusing, but we can always reevaluate down the line and besides, I think this concern will be resolved when we add a settings page for remapping shortcuts.

@hishitetsu
Copy link
Member Author

@yaira2 updated. How about this?

Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yaira2 yaira2 changed the title Feature: Added Command Palette Feature: Added a Command Palette Jul 28, 2023
@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Jul 28, 2023
@yaira2 yaira2 merged commit 2e36593 into files-community:main Jul 28, 2023
@hishitetsu hishitetsu deleted the CommandPalette branch July 28, 2023 03:52
@mdtauk
Copy link
Contributor

mdtauk commented Jul 28, 2023

image

Is there any value in only using the Semibold font for the characters you have typed into the Command Box?
image

This would match the Microsoft Store
image

@hishitetsu
Copy link
Member Author

Is there any value in only using the Semibold font for the characters you have typed into the Command Box?

I tried to find it, but could not.
https://learn.microsoft.com/en-us/windows/windows-app-sdk/api/winrt/microsoft.ui.xaml.controls.autosuggestbox?view=windows-app-sdk-1.3

@0x5bfa
Copy link
Member

0x5bfa commented Jul 28, 2023

that must be implemented by themselves.

@mdtauk
Copy link
Contributor

mdtauk commented Jul 28, 2023

Its not an automatic feature, we would have to parse the string, and provide probably Runs within TextBlocks with the characters styled.

@hishitetsu
Copy link
Member Author

Its not an automatic feature, we would have to parse the string, and provide probably Runs within TextBlocks with the characters styled.

Implemented in #13095

@mdtauk
Copy link
Contributor

mdtauk commented Jul 28, 2023

Its not an automatic feature, we would have to parse the string, and provide probably Runs within TextBlocks with the characters styled.

Implemented in #13095

Wow that was fast work, thank you. I think it does make a difference in polish and adds a confirmation to the user

@mdtauk
Copy link
Contributor

mdtauk commented Jul 29, 2023

If we can figure out how to add an Icon when the box is in the Command mode, should it match this?
image

@0x5bfa
Copy link
Member

0x5bfa commented Jul 29, 2023

yep. we can create custom control.

@mdtauk
Copy link
Contributor

mdtauk commented Jul 29, 2023

Lets keep the icon simple and not too obnoxious. It will probably require a > with a space to enter into the command mode, and switch the visuals of the AddressBar control. Backspacing will exit out of the mode.

image
image

@mdtauk
Copy link
Contributor

mdtauk commented Aug 16, 2023

@yaira2 What is the current status of the Command Palette? I saw mention about adding a button to switch to the commanding mode as well as reading the > input char?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Add command palette
4 participants