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

Customizability #68

Merged
merged 14 commits into from
Apr 3, 2015
Merged

Customizability #68

merged 14 commits into from
Apr 3, 2015

Conversation

patrickfatrick
Copy link
Contributor

Added Font Awesome and Devicons to the project, as well as functionality in main.js to support customizing the icons in the Brackets preferences file using the following format:

"file-icons.icons": [
{
"extension": "html",
"icon": "fa fa-code",
"color": "#E84D49",
"size": 16
}
]

The only required attribute is "extension", everything else will default to what's already in the extension if not specified. Next thing I'd like to do is add the ability to add your own extensions if not already supported, but for now you can customize any extension in main.js (including aliases). Oh, and I also added HAML.

UPDATE: Now includes the ability to create your own extension. If you list an extension in the preferences file that's not included in the fileInfo object, an entry will be added. See second screenshot.

brackets-customizable-icons

brackets-customizable-icons-2

Added Font Awesome and Devicons to the project, as well as
functionality in main.js to support customizing the icons in the
Brackets preferences file.
Added a check of the fileInfo object for all extensions in the
preferences file. If an extension is listed that isn’t already in
fileInfo, an entry is created for it.
@@ -1,16 +1,57 @@
define(function (require, exports, module) {
var fileInfo = {};

// Load preferences
var PreferencesManager = brackets.getModule("preferences/PreferencesManager"),
prefs = PreferencesManager.getExtensionPrefs("file-icons");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use two separate var statements here

@ivogabe
Copy link
Owner

ivogabe commented Mar 15, 2015

I'd prefer the following format:

"brackets-icons.icons": {
  "html": {
    "icon": "fa fa-code",
    "color": "#E84D49",
    "size": 16
  }
}
  • brackets-icons instead of file-icons, since this extension is called Brackets-Icons.
  • Map instead of array: less configuration, less code to read settings.

@@ -1,16 +1,57 @@
define(function (require, exports, module) {
var fileInfo = {};

// Load preferences
var PreferencesManager = brackets.getModule("preferences/PreferencesManager"),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use single quotes (')

@ivogabe
Copy link
Owner

ivogabe commented Mar 15, 2015

Thanks for the contribution! I've added some comments to the code. Can you add a new contribution.md file, which describes how to use this feature? You may add a reference to that file from the readme.

@patrickfatrick
Copy link
Contributor Author

Thanks for your comments. Very helpful as I know it's not super pretty :P. I'll work on this stuff soon (maybe not today though).

Some major changes here:

-Changed preferences to use object instead of array.

-Highly condensed the amount of code. Now everything is contained in
function applyPrefs. Basically we have an object called “icons” and one
called “userIcons”. addIcon writes to the icons object, and then
afterward the function applyPrefs updates or adds to the icons object.

-Auto-reloads preferences if they’re changed. One quirk I wasn’t able
to figure out is that I can’t get the file tree or working set to
auto-refresh. If you click on anything item inside they will refresh.

-Added a separate md file to describe usage.
 They're back in.
@patrickfatrick
Copy link
Contributor Author

Some things changed:

-Changed preferences to use object instead of array.

-Highly condensed and cleaned up the amount of code. Now everything is contained in
function applyPrefs. Basically we have an object called “icons” and one
called “userIcons”. addIcon writes to the icons object, and then
afterward the function applyPrefs updates or adds to the icons object.

-Auto-reloads preferences if they’re changed. One quirk I wasn’t able
to figure out is that I can’t get the file tree or working set to
auto-refresh. If you click on any item inside they will refresh though.

-Added a separate md file to describe usage.

Let me know what you think!

@@ -190,6 +207,26 @@ define(function (require, exports, module) {
addIcon('d', 'ion-contrast', '#960000');
addIcon('r', 'ion-ios-analytics', '#8495C0');

//Look up preferences and apply changes
function applyUserPrefs() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't fill the icons object with user icons. With this approach, reloading settings will fail when the user removed an extension from the settings file. You should check in the provider function (line 247) whether the icon exists in the userIcons object, and otherwise check it in the icons object.

@ivogabe
Copy link
Owner

ivogabe commented Mar 22, 2015

Looks good!

@patrickfatrick
Copy link
Contributor Author

That's a good point.

I've got everything in the provider function now and seems to be working well.

I might need some guidance on the best way to get it to reload though. So it does reload but you do have to click on something in the working set and file tree to get the icons to change over. (And it does now also work with removing an entry in the preferences file, from my testing). Wrapped all of the addIcon/addAlias calls in function called loadDefaults which I'm also calling in prefs.on('change') in the case of an item, that also has a default, being changed or removed. I've got some commented out code in the prefs.on('change') handler where I tried some things out but it didn't work out.

@ivogabe
Copy link
Owner

ivogabe commented Mar 23, 2015

I don't think the loadDefaults function is necessary. Can you do the following:

  • Revert commit "Changes to reloading"
  • Replace var icon = defaultIcons[ext]; [...] var data = icon ? icon : getDefaultIcon(ext); with:
var data;

if (userIcons.hasOwnProperty(ext)) {
  data = userIcons[ext];
} else if (defaultIcons.hasOwnProperty(ext)) {
  data = defaultIcons[ext];
} else {
  data = getDefaultIcon(ext);
}

Keeping the default icons size or color is really needed, it will probably give some unexpected behavior. I'll take a look at refreshing the tree when it's merged.

@patrickfatrick
Copy link
Contributor Author

I've removed the loadDefaults function, reverted back the prefs.on("change") method to where it was (let me know if you want to just get rid of that functionality entirely until we're able to get it working better, as that would take a couple seconds to take out :).

Changed the provider function to what you're looking for, so now an entire icon spec (extension, icon, color, size) will need to be specified in preferences; you can't change one or two values in an existing default icon. Made changes to the customizability readme file to reflect this change.

icon: icon,
color: color,
size: size
};
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add semicolon again

semicolon added to line 19 of main.js, removal of Font Awesome and
Devicons from first paragraph of readme.md.
ivogabe added a commit that referenced this pull request Apr 3, 2015
@ivogabe ivogabe merged commit 209adec into ivogabe:master Apr 3, 2015
@ivogabe
Copy link
Owner

ivogabe commented Apr 3, 2015

Thanks!

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.

2 participants