Resolve #345: Make icons customizable

[a-leithner]

This aims to provide a first implementation of an abstraction that could be used to customize the icons used in ICEpdf's Swing components when using them in embedding applications (i.e. fixes #345).

Some key points of the changes:

• The icon pack to be used will be loaded from Swing's UIManager using the key org.icepdf.iconpack. I took this idea from FlatLaF, its defaults can be customized this way. Technically, it should be possible to ship ICEpdf with mutliple icon packs and let the user choose their preferred one (like LibreOffice does), though I don't see a benefit investing the time right now since we only have one default pack currently.
• Though the icon pack is (dynamically) loaded from UIManager, the respective key has to be set before the first use of Images as this class will cache the icon pack in a static final field; therefore, once Images gets initialized, the icon pack cannot be changed (this is to provide consistency across the UI).
• Icon packs can freely choose the concrete pixel values for the five available icon sizes.
• Icon packs can freely choose the subset of variants they provide (e.g. FlatLaF's FlatSVGIcon, which could well be returned by a user's icon pack, automatically creates disabled icons, so these icon packs wouldn't need to set separate icons).
• The implementation of the default icon pack fails with a NPE if the requested icon could not be found.
• All places previously dealing with ImageIcon directly have been modified to only depend on Icon, as not all Icons which could be returned by icon packs have to be ImageIcons.
• The few GIF icons have been converted to PNG to be able to load them via the default icon pack.
• Icon size preferences will be preserved, but will be upgraded to use the IconSize enum upon next update.
• It is now a responsibility of the Images utility class to apply all available icon variations to a button when applying a specific icon (previously, we were manually retrieving and applying the rollover, disabled, ... variations of a button's icon everywhere we were creating buttons – this has been changed to a single call into Images, which takes the current icon pack's capability into account).
• Images takes care of retrieving the correct default icon size from user's preferences now as well.
• ImageColorIcon should probably be renamed to reflect it no longer depends on an ImageIcon.

I'm more than happy to discuss all changes and design decisions I made here. Especially the move of ImageColorIcon from extends ImageIcon to implements Icon and using composition rather than inheritance is something I'd like to hear opinions on. Also, I've placed a TODO in the constructor of AbstractColorButton, since I'm unsure if we can/should change the code calculating the preferred size the way I did (I'm also unsure of why we're changing the preferred size this way – shouldn't that component and most others calling setPreferredSize
actually override getPreferredSize?).

[pcorless]

Thank you for for putting this together. I've started working my way though it.

[pcorless]

Again this is looking pretty good. I'm thinking that keeping the icon dimension might be useful rather then just the height, thoughts?

[pcorless]

I'm starting to think maybe the height and width should be stored. Maybe this should be Images.getIconSizeDimension().

[a-leithner]

Maybe. I don't see why this particular button class needs to be rectangular rather than square — adding a getter for the icon width or the icon dimension wouldn't change much in this case, since most icons in the default pack are square and there are only very few exceptions; yet, AbstractColorButton wants to always be rectangular.

Thus, just assuming that the icon will be square seems to be "good enough"™ to me. We're already asking (by means of JavaDoc) anyone subclassing IconPack to provide square icons only.

Also note that either method, getHeightValueForIconSize or getDimensionForIconsSize, cannot be accurate unless we retrieve these values on a per-icon basis (since not even our default icon pack has exclusively square icons) which could require us reading the same bitmap multiple times.

[a-leithner]

Related: Should I make those icons which are rectangular (e.g. signature_*_lg.png) square?

[pcorless]

I'm pretty sure the code in question is related to making sure the popup shows up in the correct location. There might be something there as well for accounting for the dropdown arrow offset. I'll try a few experiments but I think as you eluded to earlier the preferred size should just be a reflection of the the child components.

[pcorless]

I looked into this a little more and I think this code can be removed and the following added at the end of void setupLayout()

Dimension buttonPreferredSize = colorButton.getPreferredSize();
Dimension dropDownArrowPreferredSize = dropDownArrowButton.getPreferredSize();
setPreferredSize(new Dimension(
        buttonPreferredSize.width + dropDownArrowPreferredSize.width,
        buttonPreferredSize.height));
validate();

[a-leithner]

So in this case, we don't need the height/size of the icon at all, do I understand you correctly? If so, do you still prefer a method retrieving the icon Dimension rather than the height only?

[pcorless]

I don't think the size will be needed at all, but I'l give it another review to double check. I think the actual size could be pulled as needed from other constructs as needed.

[a-leithner]

I've made this change now, and it should populate properly to all inheritors of AbstractColorButton; I've removed the code to manually set the preferred size on those classes too.

I couldn't get rid of Images.getHeightValueForIconSize() though as there are a few other places in SwingViewBuilder depending on it. Is that good enough?

[pcorless]

This is going to need to be fixed up. Button bar currently only has two sizes so maybe the same logic can apply for now but as is, it breaks layout for small icons.

[a-leithner]

I'm guessing you're referring to me setting new Dimension(32,32) regardless of the icon size, right? Yeah, that's a mishap; I'll fix this and implement your other comments in the following days.

[pcorless]

Yeah that's the line, similar to the other comment, maybe this size can just be set from the component.

Sound good, I'll keep an eye out for the update.

[a-leithner]

I believe that buttons should already stretch or shrink according to the size of their icons, so I've removed this method now (only three places depended on it, I believe).

[pcorless]

Thanks for the updates, I'll give it more look over but suspect it's ready to go.