1074880 - Remove file icons on file tree view

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

[Jen Fong-Adwent [:ednapiranha]]

We probably don't need images to represent the file types in the file viewer. They take up space and popular editors like Sublime just have the filename only.

Comment 1

[Paul Rouget [:paul]]

+1

Comment 2

[Jen Fong-Adwent [:ednapiranha]]

Attachment: bug1074880.patch

I think this is okay, but wanted to make sure I didn't actually blow up something else that might have been using this too.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8499669 [details] [diff] [review]
bug1074880.patch

Review of attachment 8499669 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me.  It doesn't look like your patch actually deletes the PNG, so we should do that assuming Brian agrees below.  (Bugzilla tends to fail at showing binary file changes, so I wasn't completely sure.)

Brian, did we have other consumers of project editor planned that might want these icons?  If we do, we could make this a project editor option, or we could delete them for now, and revive them from source history if we do end up wanting them later.

Comment 4

[Brian Grinstead [:bgrins]]

Comment on attachment 8499669 [details] [diff] [review]
bug1074880.patch

Review of attachment 8499669 [details] [diff] [review]:
-----------------------------------------------------------------

> Brian, did we have other consumers of project editor planned that might want
> these icons?  If we do, we could make this a project editor option, or we
> could delete them for now, and revive them from source history if we do end
> up wanting them later.

Not that I know of.  I don't feel too strongly about the icons either way - I actually kind of like them for things like distinguishing empty folders that don't have arrows by them - but I'm fine with removing them entirely and reviving them if we want them back in the future.  The binary file does seem to be deleted in this patch, so that's already taken care of.

The only change I'd request is that all references to this.icon in tree.js get removed as well.

::: browser/devtools/projecteditor/lib/tree.js
@@ -124,5 @@
>      this.elt.hidden = !visible;
>  
>      this.tree.options.resourceFormatter(this.resource, this.label);
>  
> -    this.icon.className = "file-icon";

this.icon is created and deleted elsewhere in this file - those references should be removed as well

Comment 5

[Brian Grinstead [:bgrins]]

Jen, here is a random tip for doing frontend work on the project editor - I usually just open this URL in a tab: chrome://browser/content/devtools/projecteditor-loader.xul.  Then I can use devtools on it so it makes styling changes a lot easier.

Comment 6

[Jen Fong-Adwent [:ednapiranha]]

Attachment: bug1074880.patch

Comment 7

[Jen Fong-Adwent [:ednapiranha]]

Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f2076239fe9d

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/3efeefd11c1a

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3efeefd11c1a