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

more contrast for typings folder #604

Merged
merged 4 commits into from
Jan 8, 2017
Merged

Conversation

robertohuertasm
Copy link
Member

@robertohuertasm robertohuertasm commented Jan 7, 2017

Fixes #603

Changes proposed:

  • Add
  • Prepare
  • Delete
  • Fix

Things I've done:

  • My pull request fixes an issue, I referenced the issue.

@JimiC
Copy link
Member

JimiC commented Jan 7, 2017

I would really appreciate if any alternative icons would be added as another version (i.e folder_type_typings2@2x.svg).

@robertohuertasm
Copy link
Member Author

robertohuertasm commented Jan 7, 2017

But the point of this PR is precisely make these ones a substitution of the default ones. I've tested them and I think the TS is hardly appreciated. Or do you mean keep the substituted ones as v2?

@JimiC
Copy link
Member

JimiC commented Jan 7, 2017

The point of all the work I've done was to have some art rules and guidelines from which we create the icons. Now if some ain't that pretty an alternative can be submitted as v2. Ain't that the point of the new configurable icons feature?

@robertohuertasm
Copy link
Member Author

robertohuertasm commented Jan 7, 2017

Clearly we have a different vision on this matter. Guidelines are ok but they are not a rule of thumb that must be followed eyes blinded. Even more when we're dealing with art. It's obvious that color combination was not working. Why deploying a default icon that it's not working even the user has the ability to change it at will?

For me, it's more important to provide icons that work than icons that follow the guidelines.

@JimiC
Copy link
Member

JimiC commented Jan 8, 2017

Fine. I can delete the guide I wrote then.

@robertohuertasm
Copy link
Member Author

Don't be so drastic . I'm not saying that we must get rid of the guidelines. Of course it's a fantastic tool and I'm not trying to undervalue your great work. What I'm saying is that we must apply common sense here. If the result of a specific icon built by following the guideline it's not working as expected then it's licit to change it to make it work. I'm not willing to provide default icons that are not working ok, that's all.

@JimiC
Copy link
Member

JimiC commented Jan 8, 2017

@robertohuertasm
I'll try to make my case clearer. I'm not against changing the current folder icon. I'm against replacing it entirely. We implemented the configurable icons feature to have choices, not force our subjective decisions.
It so happens that I like the current icon, why can't I have it as an alternative?
Why not make your icon v1 and the current v2? This way everybody is happy.

On another note: All icons are getting exported using Export for Screens from Illustrator. It would be nice if we could keep it that way. Surely it requires some settings tweaking. PM me to guide you through it.

@robertohuertasm
Copy link
Member Author

Agreed to publish current as v2.
I don't find this Export for Screens options. Let me know how I can do it and I'll do it.

@robertohuertasm robertohuertasm merged commit 71dfecf into master Jan 8, 2017
@robertohuertasm robertohuertasm deleted the fix-typings-contrast branch January 8, 2017 11:26
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.

New Typings folder icon should need more contrast
3 participants