Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Combine icon & kind in Outline View into a unified styleable property #1182

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Combine icon & kind in Outline View into a unified styleable property #1182

wants to merge 2 commits into from

Conversation

damieng
Copy link
Contributor

@damieng damieng commented Jun 7, 2017

Right now there is a .icon property which maps to Atom$Icons which doesn't have the icons we really want as well as a .kind property for theming I added.

This unifies the two - my new LSP-compatible icons are mapped to type-x where x is the kind property from LSP.

Should be no migration problems now as this preserves existing icon behavior for nuclide/those that used it and my usage from atom-languageclient of the now-removed kind had no impact yet.

@damieng damieng changed the title Combine the icon and kind properties into a unified styleable property Combine icon & kind in Outline View into a unified styleable property Jun 7, 2017
@facebook-github-bot
Copy link

@damieng updated the pull request - view changes

@facebook-github-bot
Copy link

@damieng updated the pull request - view changes

@facebook-github-bot
Copy link

@damieng updated the pull request - view changes

@facebook-github-bot
Copy link

@damieng updated the pull request - view changes

@wbinnssmith
Copy link
Contributor

this looks fine to me! what do you think @hansonw?

@hansonw
Copy link
Contributor

hansonw commented Jun 9, 2017

yay! When are these icons going to be in Atom?

@damieng
Copy link
Contributor Author

damieng commented Jun 9, 2017 via email

@facebook-github-bot
Copy link

@ljw1004 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@hansonw
Copy link
Contributor

hansonw commented Jun 26, 2017

@damieng should these be named "Atomicons" rather than "OcticonsPrivate"? Or at least that's what the PR suggests: atom/atom#14657

@damieng
Copy link
Contributor Author

damieng commented Jun 26, 2017

Yeah I can rename those and rebase.

@facebook-github-bot
Copy link

@ebluestein has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@damieng
Copy link
Contributor Author

damieng commented Jan 25, 2018

What's the state on this? I thought it had got merged...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants