Extend button editor to allow dynamically changing button image

[aaronayres35]

part of #1513

This PR adds an image_value trait to the custom style of the button editor (analogous to the label_value trait) to allow for dynamically changing the buttons image. It also adds a demo example to showcase both of these features.

[aaronayres35]

The added demo is hopefully useful for documentation purposes of all the button editor's functionality. Note that there is other functionality not in demos, but it may not be working or not fully implemented, eg #879

[aaronayres35]

TBH I am still sort of confused by where to use pyface.ui_traits.Image vs pyface.api.ImageResource. I understand Image is the trait type whose value needs to be an ImageResource. Nonetheless, there are occasions where I am unsure what is best (notice I used an Instance(ImageResource) at one point in this PR).

[aaronayres35]

I went with the InstanceEditor approach because trying to use ImageEnumEditor was giving me trouble/ I couldn't get things working how I wanted them to.
This demo is now a little bit showcasing the secret InstanceEditor Api as well.

Also, relating to my other comment, it seems that if I had used the Image trait correctly here, this code would be un needed as the Image trait sets the editor to ImageEditor for the ImageResource that is stored.

[aaronayres35]

@rahulporuri
It has been a bit since I looked at this PR, but from what I can tell things are working as we would want them to. I believe I had it as a WIP because I was unclear on when I should have been using ImageResource/ the Image trait type, and this was causing me some trouble getting the demo to work.
At this point, the demo is using a fancy feature of the InstanceEditor (which TBH it is probably a good thing to have an example of, but maybe not here). Maybe the demo should be rewritten somehow to avoid this, or maybe it is perfectly fine as is. It is a scenario where we want the example to showcase the "right" way to do this thing, but while I am writing the example I myself do not know what exactly is the "right" way to do this thing, so I did it a way that made sense to me that I could get working. LMK what you think

[corranwebster]

This doesn't seem right - should it be image.get_icon() or image.get_image() or something like that?

[aaronayres35]

So for Qt, image.create_icon() returns a QtGui.QIcon which can then be passed as the argument of the setIcon method on the control.

In this case on wx, the _control is a pyface.ui.wx.image_button.ImageButton and control is the control of that (ie self.control = self._control.control) which is a wx.Window. Neither of which have "icon" related methods, etc. ImageButton simply has an image = Instance(ImageResource, allow_none=True) trait, and so I set it directly to the ImageResource instance from the image trait here.

I agree it seems incorrect, and in fact it ends up working not exactly right, see the following screenshot:

ImageButton has an _image_changed method which sets

self._img = image.create_image()
self._image = self._img.ConvertToBitmap()

which seems like maybe we would want to use create_icon instead or something? I am not sure I was unable to figure this out. There is code in ImageButton.__init__ method which looks as though it is trying to figure out the size of the image and the size of the label (and that seems to work correctly - as in the above image looks as though it has the perfect button size to fit the image and label side by side). However, it seems both the image and label are getting drawn at the center which likely isn't intentional (?)
I suspect this is might be a problem in Imagebutton._on_paint

[aaronayres35]

FWIW: I tried setting orientation="horizontal" and now I see this

I think we may be using some calculated layout value in the wrong place or something of the sort. Looking into it now

[aaronayres35]

Also don't know how I'm just seeing this, but the dynamic label isn't working either!

[aaronayres35]

The label can be fixed by overwriting the _label_changed method in the traitsui.wx.button_editor.CustomEditor class to be self._control.label = self.string_value(label).

[aaronayres35]

okay I have successfully fixed this locally.

I will push the changes to this PR and open a separate PR in pyface

[aaronayres35]

I've opened enthought/pyface#932

[aaronayres35]

Thank you for the feedback! I've made the suggested changes and I think with enthought/pyface#932 things look correct now. Also using Image trait worked perfectly fine, not sure what I had been doing previously. I think some of my difficulties were tied to my attempts to use ImageEnumEditor rather than using the InstanceEditor as a makeshift EnumEditor. There is a note in the docs which says "Image enumeration editors do not use ImageResource." Perhaps this is something we want to change at some point if possible?

[rahulporuri]

Mostly LGTM with a few question around the duplicated image files. Also, there seems to be unused "enthought-icon.png" files in this PR, which can probably be removed.

[rahulporuri]

if this doesn't exist in pyface.api, let's create an issue in pyface.

[rahulporuri]

i dont see an issue for this in pyface - not sure if you had written this down on your personal to-do list.

[aaronayres35]

It actually is available from the api, I've updated the import statement on this PR

[rahulporuri]

LGTM 🎉