-
Notifications
You must be signed in to change notification settings - Fork 7.6k
[Themes] Should we allow multiple themes per extension? #8423
Comments
@peterflynn I love the idea, and that's exactly why I did https://github.com/MiguelCastillo/brackets/blob/master/src/view/ThemeManager.js#L149. So that you can have a main.less which does @import "theme1.less"; I have also added this https://github.com/MiguelCastillo/brackets/blob/master/src/view/ThemeManager.js#L450. This allows you to load a particular directory with all kinds of themes. I myself will converting my themes extension to plug into the theme manager so that I can load CodeMirror themes and others I already have. Either way, adding what you are asking isn't really too difficult at all, since the API in ThemeManager can easily be enhanced to do that. I can put together a branch that does that if you want to try it. The config processing would just happen right here https://github.com/MiguelCastillo/brackets/blob/master/src/view/ThemeManager.js#L338. |
@peterflynn Alright, so the imports won't do the multipack thing... Once themes settles down a bit, I will add logic for this if you are interested in trying it. |
@MiguelCastillo I think we need to change the package.json format now in order to give the theme a chance to express whether it is "light" or "dark" so that the rest of the Brackets UI can adapt. In #8415, I suggest that editor themes may also be able to suggest a few colors outside of the editor area. We'd need a place to put that extra metadata. I'm not actually particular about being able to bundle themes into a single extension. It is true that some themes come in light/dark pairs, but I'm not sure if users of those themes necessarily care to have both. What I do think is that the metadata needs to change to either what Peter suggests here or: "theme": {
"file": "theme.less",
"dark": true
} I don't think we want to go down the path of having package.json support By the way, Atom uses the |
Just closing the loop on this issue. I have pushed a change to support a theme object for single themes. No theme packs support, sorry @peterflynn :/ |
Oh yeah, I meant to add for posterity here a bit of discussion that @MiguelCastillo and I had on IRC. My thought was that people probably don't use more than 1 or 2 themes generally. If someone made a 10 theme pack, then we'd probably start getting comments from people who want a UI to let them hide 8 of those 10 themes from the theme manager (so they don't appear in the selection UI). Also, while theme authors sometimes create both light and dark variants of a theme, I'm not sure if the users use both. I think it makes more sense to make themes distinct extensions and have generic extension bundles be a solution if there are people who want multiple themes. I also think the pressing concern is making themes easy to browse before installation. |
This issue has already been addressed. We basically decided to only allow one theme per extension. |
This would be useful for providing theme "packs." A number of themes come in light+dark pairs, so this is probably a fairly common need. It would also be useful if any of the existing theming extensions want to convert over to just being a bundle of popular themes that plug into the core theming feature -- more than just the convenience of downloading all their old themes at once, it would let them actually update in place, removing their old menu items and morphing directly into a theme bundle.
I know we eventually want to allow bundling any set of extensions together into a single-install "pack", but that's a long way down the road still. Themes are something we could address in a much more targeted, simple way.
E.g. instead of
"theme": "<somefile>.less"
in package.json, we could do this:The text was updated successfully, but these errors were encountered: