Skip to content

Add icons for Command Explorer #1638

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

Merged
merged 4 commits into from
Jan 4, 2019
Merged

Add icons for Command Explorer #1638

merged 4 commits into from
Jan 4, 2019

Conversation

corbob
Copy link
Contributor

@corbob corbob commented Dec 7, 2018

Change refrech icon to use octicons (https://octicons.github.com)

PR Summary

Add some icons for usability.
Add configuration to toggle visibility of Command Explorer
Needing to right click isn't super intuitive.

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
  • Summarized changes
  • NA PR has tests
  • This PR is ready to merge and is not work in progress
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

Change refrech icon to use octicons (https://octicons.github.com)
Reword Refresh to specify what it's Refreshing.
Remove Insert from Command Palette.
Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

I think the setting just needs a quick name check but LGTM otherwise.

@corbob
Copy link
Contributor Author

corbob commented Dec 19, 2018

@TylerLeonhardt

From my investigation, if you navigate to View > Appearance there's two options. There's Toggle Side Bar and Hide Activity Bar. If you select Toggle Side Bar it will toggle the view of the green highlighted section. While if you select "Hide Activity Bar" it will toggle the red highlighted section. Based on this behaviour, do you still want to change it to activityBar instead of sideBar? If we don't change it from sideBar, then the description at least should change to be side bar to match the UI.

Reference screenshot:
image

@TylerLeonhardt
Copy link
Member

Given this info. Ignore all my comments. :)

Be consistent with UI
@@ -456,6 +477,11 @@
"type": "object",
"title": "PowerShell Configuration",
"properties": {
"powershell.sideBar.CommandExplorerVisibility": {
"type": "boolean",
"default":true,
Copy link
Contributor

Choose a reason for hiding this comment

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

There should've been a space after the colon (:). I opened PR #1688 to fix it. Running Invoke-Build at the end helps with that.

@corbob corbob deleted the Command-Explorer-v1.1 branch January 10, 2019 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants