Qwheeledit hide of edition widget and returnPressed forwarding

[MikeFalowski]

Our scientist asked for few things in the TaurusWheelEdit:

1. Values should be applied not only with an enter, but also with a custom button.
2. When edition widget is shown, it should not disappear on focusOut.
3. Accepting value from the edition widget should also write an attribute.

To achieve this I had to change the way the edition widget is hide in qwheel module. First of all variable _editing is set in method hiding and showing editing widget now, because earlier there were some cases, where this variable didn't changed properly (edition widget was hidden but _editing was set to True) and I didn't want to change it in the application, as it is supposed to be private. I think it should be now more consistent.

3rd point: I make this with emitting QWheelEdit.returnPressed when edition widget's returnPressed is emitted.

2nd and 3rd points are disabled by default.

In the application I made a button connected to 3 methods

    set_button = QtGui.QPushButton()
    set_button.setText("Apply")

    set_button.clicked.connect(taurus_wheel_edit.editingFinished)
    set_button.clicked.connect(taurus_wheel_edit.returnPressed)
    set_button.clicked.connect(taurus_wheel_edit.hideEditWidget)

Can you tell me if this approach is ok? Anyway, if anyone will find this useful and it's ok, feel free to merge it.

[cpascual]

Hi @MikeFalowski ,
Thanks a lot for contributing!

I have one question regarding the "WIP" in the title of the PR: by convention, we treat WIP PRs as not-ready-for-merging-yet (i.e. if you keep the "WIP" prefix in the title you indicate that there is still work you intend to do on the branch before a reviewer considers it for merging, and that you are just creating the PR to get some early advice/discussion on the implementation).

This is different from a "normal" PR which the author considers "ready" (without precluding that some further commits be done if issues arise during the review process)

So, should we understand that it is still under development?
Or should we already review it (if so, please remove the "WIP")?

[cpascual]

Regarding this part of your message...

In the application I made a button connected to 3 methods
(...)
Can you tell me if this approach is ok?

... I understand that this is in order to fulfil requirement "1" from your scientist and that this only applies to your own application, so it is up to you how you implement it.
I just have the impression that the connection totaurus_wheel_edit.returnPressed should not be necessary, and it could be handled by the connection to editingFinished

If the reason for having it is that you want the changes to be written to the control system when using a TaurusWheelEdit, I think that a more explicit call to TaurusWheelEdit.writeValue() would be better.

But as I said, it is totally up to you since that is not part of taurus but of your own app.

[cpascual]

So, should we understand that it is still under development?
Or should we already review it (if so, please remove the "WIP")?

(bump) @MikeFalowski

[cpascual]

Since we got no reply from @MikeFalowski and we are about to close the Jan19 milestone, I am moving this for the Jul19 milestone

[MikeFalowski]

Sorry for no reply. Moving it to next milestone is the best option, as I had to postpone it. I can take care of it at the end of February or in March.

[cpascual]

(bump) @MikeFalowski any news on this?

[MikeFalowski]

Hey, sorry for the delay.

Ok, so I set the WIP status, because I weren't sure is it good approach to achieve my goals that way and was thinking about changing it but for now I didn't found a better solution. I'm removing WIP.

[cpascual]

Hi @MikeFalowski , thanks for the update.

I'm removing WIP.

I'll re-enter this into the queue for reviewing ;)

[MikeFalowski]

Regarding this part of your message...

In the application I made a button connected to 3 methods
(...)
Can you tell me if this approach is ok?

... I understand that this is in order to fulfil requirement "1" from your scientist and that this only applies to your own application, so it is up to you how you implement it.
I just have the impression that the connection totaurus_wheel_edit.returnPressed should not be necessary, and it could be handled by the connection to editingFinished

If the reason for having it is that you want the changes to be written to the control system when using a TaurusWheelEdit, I think that a more explicit call to TaurusWheelEdit.writeValue() would be better.

But as I said, it is totally up to you since that is not part of taurus but of your own app.

Exactly, I was using returnPressed to apply value to the control system. Connecting writeValue seems to be more clear, so I will use it. Thanks for the advice!

[cpascual]

With the latest changes, I think that this is ready to go into the Jul19 release.
I'll merge when I get green light from Travis.
Thanks @MikeFalowski for the patience and the contribution!

[MikeFalowski]

With the latest changes, I think that this is ready to go into the Jul19 release.
I'll merge when I get green light from Travis.
Thanks @MikeFalowski for the patience and the contribution!

And thanks a lot for merging it and the help with changes!