Remove file icons on file tree view

RESOLVED FIXED in Firefox 35

Status

RESOLVED FIXED
4 years ago
6 months ago

People

(Reporter: jfong, Assigned: jfong)

Tracking

28 Branch
Firefox 35
x86
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
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.
+1
(Assignee)

Comment 2

4 years ago
Created attachment 8499669 [details] [diff] [review]
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.
Attachment #8499669 - Flags: feedback?(jryans)
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.
Attachment #8499669 - Flags: feedback?(jryans)
Attachment #8499669 - Flags: feedback?(bgrinstead)
Attachment #8499669 - Flags: feedback+
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
Attachment #8499669 - Flags: feedback?(bgrinstead) → feedback+
Status: NEW → ASSIGNED
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.
(Assignee)

Comment 6

4 years ago
Created attachment 8500631 [details] [diff] [review]
bug1074880.patch
Attachment #8499669 - Attachment is obsolete: true
Attachment #8500631 - Flags: review?(jryans)
Attachment #8500631 - Flags: review?(jryans) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/3efeefd11c1a
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/3efeefd11c1a
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35

Updated

6 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.