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

Adopt icon font in activity bar #83113

Merged
merged 5 commits into from
Oct 23, 2019
Merged

Conversation

miguelsolorio
Copy link
Contributor

@miguelsolorio miguelsolorio commented Oct 22, 2019

Related #78889

This adopts the icon font in the activity bar for our primary views:

  • Explorer
  • Search
  • SCM
  • Extensions
  • Remote

Extensions can still use custom icons and there are no visual changes:

image

cc @bpasero @sandy081

@isidorn
Copy link
Contributor

isidorn commented Oct 23, 2019

@misolori thanks for this PR. I have tried it out and it works great.
I have also tested customising activity bar colors and all is good.

The only conern I have I left a comment in the code directly. We should not reference the names of viewlets in any way, since this is a complete hack. I am aslo not sure why is this needed? Maybe you can explain the issue.

@miguelsolorio
Copy link
Contributor Author

@isidorn let me know if this is good to merge in. Later on I can work on seeing if we can add some type of extension property to ActivityActionViewItem so we can easily identify extensions.

@isidorn
Copy link
Contributor

isidorn commented Oct 23, 2019

Yes, this is better now and we can merge. Thanks a lot
However please create a debt item, assign it to November and ping @sandy081 and me on it. So we see how we make it nicer.

@miguelsolorio miguelsolorio merged commit e198750 into master Oct 23, 2019
@miguelsolorio miguelsolorio deleted the misolori/icon-font-activitybar branch October 23, 2019 16:56
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants