Bug 53461 - Allow AbstractNode to read also .png icons
Allow AbstractNode to read also .png icons
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Nodes
4.x
All All
: P2 (vote)
: 5.x
Assigned To: Petr Nejedly
issues@platform
OSLO bin_comp
: API, API_REVIEW_FAST
: 52794 (view as bug list)
Depends on:
Blocks: 53456
  Show dependency treegraph
 
Reported: 2005-01-15 00:32 UTC by Peter Zavadsky
Modified: 2008-12-22 19:25 UTC (History)
3 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
Edited diff of the hack (1.50 KB, patch)
2005-01-15 00:32 UTC, Peter Zavadsky
Details | Diff
Proposed implementation (7.26 KB, patch)
2005-06-03 15:04 UTC, Petr Nejedly
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Zavadsky 2005-01-15 00:32:09 UTC
We need to be able to load png icons for nodes too.
There is a diff of our hack.

But the fix could be made that way, which wouldn't
affect NB preference, and allow modify the default
behaviour.

I think the solution could be that the field icons
(in the AbstractNode class) is read from Bundle
(as a list of the strings) instead of hardcoded in
the source. Then it would be possible to brand
that list to our needs. This is just a suggestion,
any solution which you prefer more is welcome
Comment 1 Peter Zavadsky 2005-01-15 00:32:56 UTC
Created attachment 19716 [details]
Edited diff of the hack
Comment 2 Petr Nejedly 2005-01-20 11:05:54 UTC
*** Issue 52794 has been marked as a duplicate of this issue. ***
Comment 3 Petr Nejedly 2005-01-20 11:13:13 UTC
No need to look up .png variants, just rename your .png file to .gif
and keep current lookup mechanism.
Comment 4 Marco Walther 2005-04-22 20:18:08 UTC
Please NO! 
 
Don't just change the suffix on files to avoid having to change the code. 
Otherwise I would have to suggest to change all the *.java files into *.c;-) 
 
(And that suggestion is comming from a product which heavily depends on 
suffixes for it's own working:-() 
 
Thanks, 
-- Marco 
Comment 5 Jesse Glick 2005-05-21 19:47:32 UTC
The proposed diff just makes things worse, I think. Prefer an API change.
Deprecate setIconBase and introduce something else, e.g.
setIconBaseWithExtension which would be almost the same but insert infixes
rather than suffixes.

(The suggestion to load icon paths from bundles may be unnecessary since you can
already brand icons loaded from AbstractNode or more generally from
Utilities.loadImage. You cannot change the extension but that probably won't
matter much - we should just replace all of our GIFs with PNGs in bulk anyway.)

DEFECT is not appropriate for this since you are not prevented from providing
PNG icons. You can return whatever Image you like from getIcon and
getOpenedIcon, which is the API. Rather, there is a convenience facility that
lets you write slightly less code (*) for AbstractNode subclasses, which works
with *.gif as documented. So this is an RFE to extend that convenience to other
file extensions.

(*) One method call rather than one or two 1-line method overrides.
Comment 6 Peter Zavadsky 2005-05-23 17:31:15 UTC
The diff is just a way we solved that.
You can come up with a solution which is more suitable for NB, and which could
be more efficient (e.g. from performance point of view).
Comment 7 Petr Nejedly 2005-06-01 13:22:44 UTC
I'm going to implement this using the API change way.
Comment 8 Petr Nejedly 2005-06-03 14:57:07 UTC
OK, I'm asking for the API review of the change.

I'm adding a method AbstractNode.setIconBaseAndExtension(String base, String ext)
which sets up internal structures to support any extension of icon file.
The original method uses the default extension ".gif"
Without this change, one can't that easily use e.g. PNG icons

This change is in line with current Nodes architecture.

I'll attach the diff of changes including javadoc and api-changes entry.
Comment 9 Petr Nejedly 2005-06-03 15:04:39 UTC
Created attachment 22482 [details]
Proposed implementation
Comment 10 Jesse Glick 2005-06-03 18:09:15 UTC
I would have a slight preference for

  setIconBaseWithExtension("org/foo/something.png");

over

  setIconBaseAndExtension("org/foo/something", ".png");

since the latter looks a bit artificial to me, and is less likely to give you a
search hit if you are grepping sources for usages of some icon. The change would
mean a little bit more work to implement: you need to scan the supplied string
for the last '/' (if any), then scan the suffix after the slash for the last '.'
(if any), which should be the infix insertion point. (Similar logic is used
widely, e.g. for the 'nbresloc' URL protocol handler.)

You could consider also deprecating the old setIconBase, and/or doing a bulk
replace of known sources to use the new method, but neither are necessary.
Comment 11 Jaroslav Tulach 2005-06-09 10:43:15 UTC
Can a test be written before integration? 
Comment 12 Petr Nejedly 2005-06-14 17:15:05 UTC
Finally implemented as suggested by Jesse. Available @since o-o-nodes 6.5
Extensive test writen and passing.

cvs/openide/nodes/src/org/openide/nodes/AbstractNode.java,v1.2


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo