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

File and folder icon theme feature #3200

Closed
wants to merge 1 commit into from
Closed

File and folder icon theme feature #3200

wants to merge 1 commit into from

Conversation

pflannery
Copy link

Relates to #211

Adds ability to create color and icon themes using css.
Adds ability to create an icon theme (css only).
Adds ability to switch icon theme's independently of color themes
Doesn't break current textmate themes

new-icon-theme-option

Examples of icon themes mixed with different color themes: (icons probably not to be included in the final merge)
dark-plusiwth-icons
untitled
monokai-icons-with-quiet-light

@msftclas
Copy link

Hi @pflannery, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

@pflannery
Copy link
Author

I couldn't find the svg icons for the "file and open folder" so I didn't add any css for them.

This would be the base css classes I would of liked to of added to https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/files/browser/media/explorerviewlet.css

/* explorer icons */
.explorer-folders-view .folder-icon {
    padding-left: 22px;
    background: url(media/folder.svg) 1px 4px no-repeat;
}

.explorer-folders-view .expanded .folder-icon {
    background: url(media/folder_open.svg) 1px 4px no-repeat;
}

.explorer-folders-view .file-icon {
    padding-left: 22px;
    background: url(media/file.svg) 1px 4px no-repeat;
}

.explorer-working-files .working-files-item {
    padding-left: 22px;
    background: url(media/file.svg) 1px 4px no-repeat;
}

Themes would only need to override the background-url for the file icon using the file-type-{ext} class.

@joaomoreno joaomoreno added this to the Backlog milestone Feb 22, 2016
@msftclas
Copy link

@pflannery, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@bpasero
Copy link
Member

bpasero commented Feb 29, 2016

@aeschli is it possible with our theming story to add global CSS rules that alter things outside the monaco editor? I would think not?

@aeschli
Copy link
Contributor

aeschli commented Feb 29, 2016

@bpasero Yes sure. The theming CSS rules are all global and can be used anywhere. But there's a limited set of standardized colors that TextMate themes define and they are editor related. Custom colors are also possible, of course.

@bpasero
Copy link
Member

bpasero commented Feb 29, 2016

@pflannery currently you cannot really theme anything outside the editor. @aeschli please chime in unless I am correct with that statement.

@JoshAddington
Copy link

Is this PR going to be merged? It sounds like @aeschli is saying that this is possible. After getting used to file icons in Sublime and Atom, it's a bit of a dealbreaker not to support them.

@bpasero
Copy link
Member

bpasero commented Mar 9, 2016

@JoshAddington we can merge this in but it is NOT possible to add file icons to the explorer via theme extensions.

@pflannery
Copy link
Author

@bpasero @aeschli I see now how theming worked with tm.

This evening I added the rework css parser which allows theming using css. This doesnt break tm themes.
I've added an example 'monokai with icons' theme.
The css selectors are controlled by a look up map and only those selectors can be used in css themes. ([attr=value] is allowed)
I've also moved the filename to attributes in the file items so we can detect gulp, grunt and bower files etc...

These are the allowed css selector\property definitions.

https://github.com/pflannery/vscode/blob/file-folder-icons/src/vs/workbench/services/themes/common/themeCssService.ts

Example theme using css for editor and explorer

https://github.com/pflannery/vscode/blob/file-folder-icons/extensions/theme-monokai-with-icons/themes/theme-monokai-with-icons.css

Example of css monokai theme with icons:

untitled

@nelsonlaquet
Copy link

Nice!

One thing though, I don't know if it's a good idea to tie icons directly in with the theme. I know that it works now, and it'll likely be the best solution until more work is done on Microsoft's side. However, I I find it likely that a person could really like a particular theme, but the theme's author either doesn't support icons, or supports icons that the user doesn't like.

Since the presence of icons doesn't affect anything structurally, I wonder how feasible it would be to move the concept of an icon pack outside of the theme infrastructure; so users can mix and match themes with icons. Of course, that would require much more code than simply hijacking themes.

@pflannery
Copy link
Author

if your happy for this to happen then

  1. we can add plugins like css whitespace for optional nesting and rework var plugin for css-next variables etc etc...
  2. I can change the filename attribute to data-filename= for html validation
  3. add some unit tests
  4. squash to one commit

@nelsonlaquet yeah would be nice to be able swap out the icons only. I still think themes could still choose to provide their own icons to match their colouring styles.

@Tyriar
Copy link
Member

Tyriar commented Mar 10, 2016

Super excited to see some progress on this!

I don't think themes are the right place to put icons though. The wrong icons would ruin a good theme and it would probably end up spawning a bunch of derivative themes just for icons: monokai-flat-icon, monokai-someother-icon, monkai-dimmed-flat-icon, ... You can imagine how it would get out of hand.

@pflannery
Copy link
Author

@Tyriar your right. I can also see many derivatives either way its done i.e. dark-web-icon-pack, high-contrast-web-icon-pack, light-web-icon-pack etc...
also clashes like .cs extensions for c# or coffee script would mean csharp-web-icon-pack etc...

I've always thought of the term theme to mean colours, icons, wallpapers and sounds (sounds not being relevant here).

I think we could setup themes so that they declare what components they contain. i.e. icons, syntax or both.
Then the ide allows us to swap out these components and also giving us the choice to turn off icons all together.

For the global dark, light, or high contrast themes we could eventually drop these in favour of css controlling the entire ide.
These global's could then be turned in to generic default, dark, light, or hc syntax themes.

@Code-DJ
Copy link

Code-DJ commented Mar 11, 2016

@pflannery I glanced over and found you are using png. Will this work well on retina display?

@pflannery
Copy link
Author

@Code-DJ it's really only in order to get it to work and show its working. The example icon pack will be removed before it's merged.

@pflannery
Copy link
Author

@bpasero I've tried to add a unit test but I cant seem to run tests.

When using the launcher it doesn't run my test.

When using ctrl+shift+t I'm told weak is not a 32bit program. Which is weird because I can build and launch vs code with no errors.

Is there something I need to do on windows for this?

@bpasero
Copy link
Member

bpasero commented Mar 13, 2016

@pflannery you should be able to run the tests from the command line easily by typing scripts/test.sh

@pflannery
Copy link
Author

you should be able to run the tests from the command line easily by typing scripts/test.sh

thanks @bpasero . I was trying to use the launcher or test task but they aren't working.

Debugging is still a bit of chore but have it working like this

scripts\test.bat --debug-brk -f ^cssTheme

@pflannery pflannery changed the title Adds file ext class names for explorer icon support File and folder icon theme feature Mar 13, 2016
@pflannery
Copy link
Author

@aeschli @bpasero I didn't want to just go off and do this without first discussing with you.
Now that there is the new json theme capability I think we should have strategy pattern for each theme file format (json, tm and css).
The current loading and parsing code would be exactly the same as now but will be encapsulated and easier to test and support for each theme file format.

How's that sound?

@be5invis
Copy link
Contributor

@pflannery How about supporting themeing more UI components, like the color of the status bar?

@bpasero
Copy link
Member

bpasero commented Mar 15, 2016

@pflannery I appreciate your work but we are currently not planning to support theming outside the editor. We might revisit this post GA when we decide our 6 months roadmap and keep you in the loop.

@pflannery
Copy link
Author

@bpasero I hope the plan will include

  • all theme's to get the same extensibility as what the high contrast theme has now.
  • able to use css to create theme's (in this PR)
  • extensible folder and file icon themes (in this PR)
  • resource colouring (data attributes added in this PR make it possible)
  • allow users to override theme style's using a preference file in .vscode i.e. contained in ./.vscode/theme.css (json|tm|etc..)

Would be a shame to be stuck with editor themes via abstracted config json\xml files

@bpasero
Copy link
Member

bpasero commented Mar 15, 2016

@chrisdias fyi on the last comment for planning

@bgashler1 bgashler1 added the ux User experience issues label Mar 16, 2016
@bgashler1 bgashler1 added the feature-request Request for new features or functionality label Mar 16, 2016
@ironcladlou
Copy link
Contributor

Sad to see this one go. Somehow the monochrome and same-weighted text in the folder view of VSCode makes it extremely difficult for me to read compared to all other editors which have either colorization or at least bold weights for folders to distinguish them from files. I'll be seeking a hack in the meantime.

Edit: hack discovered

- adds ability to create themes using css
- adds new preferences for icon theme selection
- adds icon theme menu option with no icon option

- adds explorer and working file renderer tests
- adds theme service tests
- adds css theme provider tests
@glen-84
Copy link

glen-84 commented Apr 9, 2016

This seems very specific to one area of theming. In my opinion, you should be able to theme (apply CSS to) any part of the application. This is very easy to do when you're using something like Electron.

Related: #459

@nelsonlaquet
Copy link

@glen-84 that could cause issues and massive incompatibilities between plugins. Sure; it would open up the potential for a lot of awesome things, but I get the feeling the VSCode dev team wants to keep things as locked down as possible to ensure a consistent and usable experience.

@glen-84
Copy link

glen-84 commented Apr 16, 2016

@nelsonlaquet You're no fun! 😁

But seriously, I don't believe that custom CSS would cause massive incompatibilities between plug-ins. People love customizing things, and it builds community.

I really hope that Microsoft doesn't do the Windows thing again, where you literally have to edit registry values just to change the font.

@HunterMitchell
Copy link

HunterMitchell commented May 10, 2016

@bpasero So, what is the current stance on #211 and this (#3200)? Can we expect Icon support within the file explorer? Honestly, everything is monotone and it takes me a while (in projects with many files) to actually distinguish between files and folders.

@Tyriar
Copy link
Member

Tyriar commented May 12, 2016

@FEARCODE see #211 (comment) for a recap of the current situation. I'd love to see it added personally.

@HunterMitchell
Copy link

@Tyriar I agree. I am not sure how they went about their research, but a monotone list of names is hard to look at when I have a ton of files. Also, his comment on "multi-colored" distracting icons is false. As pictured here, these icon DRASTICALLY improve the speed at which i can find my folder/files. #211 (comment)

I think that themes should also control what the icons look like and i do agree there should be an option to disable them.

@tht13
Copy link
Contributor

tht13 commented May 26, 2016

Since we are now using electron 0.37 (Based on chrome 49) could we not use css properties(variables) to solve the icon url path issue, or even all of the css extensibility.
see: https://www.chromestatus.com/feature/6401356696911872

As the properties can also be accessed through JavaScript, something then like a .json file could be used to define the properties and then VSCode applies them to the UI. This would prevent devs using css to alter things they shouldn't.

@pflannery
Copy link
Author

@tht13 JSON is about an hours work away in the PR but I've stopped whilst we wait for the road map.

This would prevent devs using css to alter things they shouldn't.

I don't agree on your comments regarding CSS. If I don't like an extension then I just remove it.
Besides we can preview a icon\color themes before committing to using them.
On top of that this PR ensures CSS is only used for icons in the explorer.

Atom is a good example of letting designers build themes using less. Their approach is a modern approach. Example
I've used rework in this PR and not against less or sass. I want to keep this minimal as possible as there are many other people's preferences which can be added over time.

@pflannery
Copy link
Author

I've just looked at the amount of changes that have been since this PR was active and I've decided to abandon this PR as it's too old now.

@pflannery pflannery closed this May 26, 2016
@tht13
Copy link
Contributor

tht13 commented May 26, 2016

@pflannery I didn't realize you had implemented it in such a way, nice!

Using native css variables was just a selection as they can then be manipulated from the javascript side more easily. I hope this comes back soon in a new PR, would be really cool to have!

@kruncher
Copy link

kruncher commented Jul 9, 2016

@pflannery what is the name of the third theme in the first screenshot?

@pflannery
Copy link
Author

@kruncher It was an example theme (example-2) I made with this PR to show this change working

css
https://github.com/pflannery/vscode/blob/1275418cf0af648e8dbea2cdd412e2f47b9d25b3/extensions/theme-example-icons/themes/example-2-colors.css

@kruncher
Copy link

@pflannery it looks awesome; I'd totally use that as my theme! I love the icon style as well.

Thanks, I will see if I can modify an existing .tmTheme file to make this work.

@pflannery
Copy link
Author

@kruncher icons are here if you want them https://github.com/pflannery/vscode/tree/1275418cf0af648e8dbea2cdd412e2f47b9d25b3/extensions/theme-example-icons/themes/example-2-icons

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality ux User experience issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.