Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extend button editor to allow dynamically changing button image #1566
Extend button editor to allow dynamically changing button image #1566
Changes from 11 commits
d0c18d1
ee8dd05
93527d6
10ccf08
d73b7f9
eb12a08
8b8aafa
0ca454f
ba6fd76
271e0e1
f3f5fc8
d4b1d71
48a9136
a6f868c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this doesn't exist in
pyface.api
, let's create an issue in pyface.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont see an issue for this in pyface - not sure if you had written this down on your personal to-do list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually is available from the api, I've updated the import statement on this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem right - should it be
image.get_icon()
orimage.get_image()
or something like that?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for Qt,
image.create_icon()
returns aQtGui.QIcon
which can then be passed as the argument of thesetIcon
method on the control.In this case on wx, the
_control
is apyface.ui.wx.image_button.ImageButton
andcontrol
is the control of that (ieself.control = self._control.control
) which is awx.Window
. Neither of which have "icon" related methods, etc.ImageButton
simply has animage = Instance(ImageResource, allow_none=True)
trait, and so I set it directly to theImageResource
instance from theimage
trait here.I agree it seems incorrect, and in fact it ends up working not exactly right, see the following screenshot:
![Screen Shot 2021-04-20 at 9 22 20 AM](https://user-images.githubusercontent.com/36972686/115431238-ee104f00-a1b9-11eb-965f-8418af329346.png)
ImageButton
has an_image_changed
method which setswhich 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW: I tried setting
orientation="horizontal"
and now I see thisI think we may be using some calculated layout value in the wrong place or something of the sort. Looking into it now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also don't know how I'm just seeing this, but the dynamic label isn't working either!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The label can be fixed by overwriting the
_label_changed
method in thetraitsui.wx.button_editor.CustomEditor
class to beself._control.label = self.string_value(label)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay I have successfully fixed this locally.
I will push the changes to this PR and open a separate PR in pyface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened enthought/pyface#932