FlatLaf: use NetBeans folder icons in NB Explorer

[DevCharly]

Currently there is a mix between NetBeans style icons and FlatLaf outlined folder icons in many views (e.g. in Projects or Files views), which does not look good. This PR replaces the FlatLaf folder icons with NetBeans style icons in NB Explorer component.

@eirikbakke @neilcsmith-net
ATM this PR uses icons from module o.n.swing.plaf in module o.n.swing.laf.flatlaf.
It works, but I'm not sure whether this is allowed?
Or is it better to copy the icons?

Before:

After:

Dark Before:

Dark After:

New Project before:

New Project after:

[mbien]

I actually liked the new icons :(

[neilcsmith-net]

Agree with @mbien Now we've already had a release with these I think we should keep them.

[eirikbakke]

I agree with @DevCharly here; the rest of NetBeans has a particular style of icons which should be used consistently throughout the app, so this PR is an improvement. (In fact I have this change made already in my private NetBeans build...)

[eirikbakke]

I have a similar patch in my own NetBeans build, but in that case I used "Tree.closedIcon" and "Tree.openIcon" as the UIManager keys instead of "Nb.Explorer.Folder.icon" (EXPLORER_FOLDER_ICON) and "Nb.Explorer.Folder.openedIcon"(EXPLORER_FOLDER_OPENED_ICON). Perhaps the former is better, since it is probably applied in both the NetBeans explorer components as well as in the more general JTree component?

[eirikbakke]

Yeah, I checked this and the keys actually need to be "Tree.closedIcon" and "Tree.openIcon", otherwise in some cases you get a mix of folder icons like this:

(Probably because some parts of NetBeans explicitly looks for these icons in the JTree UIManager properties.)

[eirikbakke]

ATM this PR uses icons from module o.n.swing.plaf in module o.n.swing.laf.flatlaf.
It works, but I'm not sure whether this is allowed?
Or is it better to copy the icons?

I've been doing the same in my private NetBeans build for over a year, with no problem. If it works, I think it's fine (since copying the icons has its own downsides).

[mbien]

ok. so can we make this optional? e.g via "Use FlatLAF icons" checkbox or flag?

[eirikbakke]

ok. so can we make this optional? e.g via "Use FlatLAF icons" checkbox or flag?

This is one icon out of hundreds; perhaps just make it configurable via FlatLAF.properties?

(I think some trick was needed to make SVG icon loading work from FlatLAF.properties rather than from FlatLFCustoms. Here's the code I used to achieve this myself: https://gist.github.com/eirikbakke/5587bd548a668cb0588c3fdc7d9e9f48 . Though perhaps FlatLAF has its own mechanism to allow this.)

[neilcsmith-net]

I was talking to @geertjanw about release notes, screenshots, etc. and one of the things he mentioned with that was the positive fact that the default UI has begun to stabilise after some period of flux. This kind of got into the NB17 release accidentally without much discussion, and we should be careful of such in future, but I'm not sure putting it immediately back again as the default in NB18 is a good idea.

[mbien]

ok. so can we make this optional? e.g via "Use FlatLAF icons" checkbox or flag?

This is one icon out of hundreds; perhaps just make it configurable via FlatLAF.properties?

sure why not - whatever works best. There is also still a lot of empty space in the FlatLaf option dialog. And this isn't really about one icon btw, the file dialogs do use more FlatLAF icons. There is a screenshot on the end of the original PR #5298

[eirikbakke]

This kind of got into the NB17 release accidentally without much discussion, and we should be careful of such in future, but I'm not sure putting it immediately back again as the default in NB18 is a good idea.

With cosmetic changes like this I think we just need to discuss whether each PR is a net improvement or not; stability is mainly a concern for API changes, or UI changes that affect users' workflows.

There is also still a lot of empty space in the FlatLaf option dialog.

We may have different philosophies here, but I think having a large number of explicitly exposed options, about very minute details of the UI, reduces usability (due to decision fatigue). But the ability to easily change FlatLAF.properties, which is more obviously an "advanced" feature, is very nice. It could even be the basis for a "skinning" feature in the future.

[mbien]

We may have different philosophies here, but I think having a large number of explicitly exposed options, about very minute details of the UI, reduces usability (due to decision fatigue).

no i do understand this problem. Adding options for everything is not my first choice, there is a budget for that too. But UI topics specifically have the tendency to be very user specific. I still see users having nimbus LAF enabled from time to time for example (which is to my eyes super ugly and also somewhat broken in NB). And we clearly have a situation here of some liking the new icons and caring less about consistency (or rising valid points of not flip flopping between two states from release to release), and others caring more about consistency and/or maybe not liking the new icons. This maps beautifully to a checkbox. We can make both camps happy - that is all I am trying to say :)

[eirikbakke]

Yeah, agree that visual preferences are very user-specific. (If you remember the old Winamp MP3 player, it was hugely popular in part because people could change its skin...)

And we clearly have a situation here of some liking the new icons and caring less about consistency

I think what's happening is that some people see the new folder icon, which has a more modern style, and assume that it's part of some broader effort to migrate all the icons to the new style. Or perhaps not; do you really prefer the "mix" of styles yourself?

But I'm fine with new preferences as long as someone really think they are justified.

[Chris2011]

We should try to change all icon styles like an icon theme at once or not. First we started redrawing NetBeans icons with new ones, which is great and I really appreciates it, but we had mixed really nice flat looking icons wich old not so flat looking icons. Now we added another style to the icons like outlined. IMHO either we should to switch to old, to flat or to outlined all at once or not. But changing icons, step by step looks in some cases really weird and not really wanted.

I know that this is a big effort to do but maybe we should start with this first and not trying to change icons in those areas and next in those, etc. My 2 cents.

We have still very very old styled icons with shadows and shapes which also not fits into a flat style today (doesn't matter it is FlatLaF or not) We should have different icon types like "Shaped", "Flat", "Flat outlined", "Material", "Glass" whatever the users wants, but we need to have a mapping 1 by 1 to each icon first to change this with a file e.g.

[Chris2011]

Also for options, for me I'm a huge fan to make everything configurable. Sure we try to make everything right for the most of the people, but you can't. Neither for style nor for other defaults. And if you see VS Code or JetBrains how much they have to be configured, it is just wow. But this is a whole other discussion. It just came into my mind as there was a mention here about philosophy.

And please don't get me wrong, I also like the new icons, but it is really hard to see them in between all of the other old icons. I also like it more colorful so what about colored outlined icons? Next thing. So for example in VS Code the default icons, I don't like because it was hard for me to see a difference between files sometimes and folders (we are all different in our heads) because folders doesn't have icons by default. Switched to material icon theme and everything was fine and not just for the type of choice but for a better user experience on my side.

[Chris2011]

Yeah, agree that visual preferences are very user-specific

I would say a clear yes and no. Of course, we have our own preferences and also there are some "standards" out there which where tested (most of the time) and where someone wrote about it like studies, etc. So I would say a good user interface can have both: a really good and stable base + the user preferences

[neilcsmith-net]

stability is mainly a concern for API changes, or UI changes that affect users' workflows.

I would have said the same thing before conversation with Geertjan on it - hadn't considered stability of the defaults for screenshots, user familiarity, etc.

However, as that won't be relevant until NB18, and thinking about this more while using the IDE over the last few days, I'm coming around to the argument for switching it back now and thinking about icon changes more thoroughly and consistently later. With the changes to use Tree icons @eirikbakke mentions above.

[DevCharly]

Thanks for all the feedback.

Have updated the implementation.
It now sets Tree.closedIcon and Tree.openIcon (thanks @eirikbakke).

And the icons are now set in FlatLaf.properties (was class FlatLFCustoms before),
which allows overriding them in custom properties ("Edit custom properties" button in FlatLaf Options dialog).

@mbien add this to custom properties to change them back to FlatLaf outlined icons:

Tree.closedIcon = com.formdev.flatlaf.icons.FlatTreeClosedIcon
Tree.openIcon = com.formdev.flatlaf.icons.FlatTreeOpenIcon

[neilcsmith-net]

Thanks @DevCharly Locked and unlocked to force a CI build (needed to trigger if you change the labels).

Only thing that might want changing now is that we should probably use the localized form of loadImage() and pass in true?

Aside : Would be great if it was possible to link images directly in the FlatLaf properties file loading, although I guess that would add some complications, particularly as we'd want to specify the class (ImageUtilities) that loads them. Maybe an option to include a second String argument (eg. path) on the line to pass into the icon constructor?

[mbien]

@DevCharly awesome, thanks! Tested it and it works great. I was a bit worried about the folder icon in the file chooser but it remained unchanged, which is good for consistency reasons I believe (otherwise the problem would just have moved from one place to another).

[neilcsmith-net]

Only thing that might want changing now is that we should probably use the localized form of loadImage() and pass in true?

Ignore that - looks like the other LAF code doesn't either from https://github.com/apache/netbeans/blob/master/platform/o.n.swing.plaf/src/org/netbeans/swing/plaf/util/UIUtils.java#L90

[DevCharly]

Aside : Would be great if it was possible to link images directly in the FlatLaf properties file loading, although I guess that would add some complications, particularly as we'd want to specify the class (ImageUtilities) that loads them.

That could be solved by specifying an "icon loader". E.g.:

FlatLaf.iconLoader = org.openide.util.ImageUtilities.loadImage()
Tree.closedIcon = org/netbeans/swing/plaf/resources/hidpi-folder-closed.png

Then FlatLaf could detect whether the icon is specified as a path and use the icon loader to load it.

Maybe an option to include a second String argument (eg. path) on the line to pass into the icon constructor?

Also a good idea.

I'll implement those ideas in one of the next FlatLaf versions. Thx