Skip to content
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

Esc should cancel the in-going edition of a layer parameter value #2411

Closed
rodolforg opened this issue Nov 9, 2021 · 20 comments
Closed

Esc should cancel the in-going edition of a layer parameter value #2411

rodolforg opened this issue Nov 9, 2021 · 20 comments

Comments

@rodolforg
Copy link
Contributor

Is your feature request related to a problem? Please describe.
I'm always frustrated when I'm typing a new value for a parameter and I gave up on this edition, but I can't cancel it by keyboard.

Describe the solution you'd like
By pressing Esc I should be able to cancel my in-going edition on the Value column of the Parameters Panel as I'm able to when editing a layer name in the Layers Panel.

@nickleus27
Copy link
Contributor

Hi @rodolforg ! Is anyone working on this issue? If not I am interested.

@ice0
Copy link
Collaborator

ice0 commented Dec 4, 2021

Hi @nickleus27!
Assigned it to you.

@nickleus27
Copy link
Contributor

@ice0 I am looking in layertree.cpp with no luck. Also, the only GDK_KEY_Escape case is in state_bline.cpp in the event_key_release_handler method, and in selectdraghelper.h in the process_key_release_event method. I cannot see how these methods connect with the layer_tree?

@ice0
Copy link
Collaborator

ice0 commented Dec 4, 2021

I think Rodolfo wrote about this panel:

Screenshot_2

it should be located in the dock_params.cpp file.

Also, the only GDK_KEY_Escape case is in state_bline.cpp in the event_key_release_handler method, and in selectdraghelper.h in the process_key_release_event method. I cannot see how these methods connect with the layer_tree?

I think the Esc key is currently not connected to this panel and that is the reason why you cannot find it.
As for me it's best to start with a simple gtkmm app here, just to test how it works out of the box.
Also you can check the gtk-demo application, it should be located in /opt/homebrew/bin/gtk3-demo.

@nickleus27
Copy link
Contributor

@ice0 awesome, will do. This will help get me going 👍

@nickleus27
Copy link
Contributor

Hi @rodolforg ! ice0 recommended i look in dock_params.cpp in regards to this issue. After looking there for a while I ended up in cellrenderer_value.cpp . Any advice if I am looking in the right area on this issue? I see that on_editing_done() in cellrenderer_value.cpp is called when the you press enter in the "value" cell of the param table. But, it does not respond when you press esc.

@nickleus27
Copy link
Contributor

@ice0 I have been looking over the gtk3-demo. I have been looking at the editable cells demo, and the GtkCellRenderer::editing-started signal. I am having a hard time figuring out why the cell responds to say the "return" key but not the "escape" key in the param "value" column in synfig.

@ice0
Copy link
Collaborator

ice0 commented Dec 6, 2021

@nickleus27

Please check bool on_event(GdkEvent *event) method (cellrenderer_value.cpp). It looks like it stopping propagation of the event.

P.S. Rodolfo wrote that he will be busy at work for a while, this is the reason why he doesn't respond.

@nickleus27
Copy link
Contributor

@ice0 Thank you for your response. The GDK_KEY_RELEASE gets triggered when you press esc in the bool on_event(GdkEvent *event) method but it does not trigger void on_editing_done() method in the same class. This issue might be out of the scope of my understanding. It is not clear to me how on_editing_done() is called from on_event(*event)

@nickleus27
Copy link
Contributor

I guess it gets called from signal_edited() ?

@rodolforg
Copy link
Contributor Author

Hi @nickleus27 sorry my absence, I'm really 'suffocated' with my job due to end of year.
But take a look here #1208, maybe it helps you.

@nickleus27
Copy link
Contributor

@rodolforg Thank you for your response, this definitely helps.

@nickleus27
Copy link
Contributor

nickleus27 commented Dec 7, 2021

I am still having a hard time understanding how the program branches to on_editing_done() or on_value_editing_done() from what I think is on_event(Gdk::Event *event) returning true. I read through #1208 and it seems like bool Widget_Vector::on_key_press_event(GdkEventKey* key_event) in widget_vector.cpp is the equivalent to on_event(Gdk::Event *event) in cellrenderer_value.cpp.

@rodolforg
Copy link
Contributor Author

Several (all?) gtk signal callbacks/slots that are supposed to return a bool value that, if true, means the event shouldn't handled anywhere else - you did what it should be done.

The on_event is a general signal slot (for 'any' event signal), on_key_press_event is specific for key-press signal. So, on_event, yes,, can be used to handle the key-press signal.

The Esc key pressing should call the editing_canceled signal. https://docs.gtk.org/gtk3/signal.CellRenderer.editing-canceled.html

@ice0
Copy link
Collaborator

ice0 commented Dec 7, 2021

I have been looking over the gtk3-demo. I have been looking at the editable cells demo, and the GtkCellRenderer::editing-started signal. I am having a hard time figuring out why the cell responds to say the "return" key but not the "escape" key in the param "value" column in synfig.

This is the expected behavior (docs).

Implementations of Gtk::CellEditable are responsible for emitting this signal when they are done editing, e.g. Gtk::Entry emits this signal when the user presses Enter. Typical things to do in a handler for signal_editing_done() are to capture the edited value, disconnect the cell_editable from signals on the Gtk::CellRenderer, etc.

Unfortunately, they didn't describe how to intercept the Esc key, so it just should work out of the box.
I think the reason it doesn't work is because it is not a standard widget in Synfig.
As you may notice in CellRenderer_ValueBase::start_editing_vfunc:

value_entry = manage(new ValueBase_Entry());
value_entry->set_path(path);
value_entry->set_canvas(get_canvas());
value_entry->set_param_desc(get_param_desc());
value_entry->set_value_desc(get_value_desc());
value_entry->set_child_param_desc(get_child_param_desc());
value_entry->set_value(data);
value_entry->set_parent(&widget);
value_entry->show(); // in order to enable "instant"/"single-click" pop-up for enum comboboxes
value_entry->signal_editing_done().connect(sigc::mem_fun(*this, &CellRenderer_ValueBase::on_value_editing_done));

Here Synfig creates a new ValueBase_Entry widget and display it on top of the standard cell widget.
So this widget just doesn't know how to handle keypress events.

Also, if you check the ValueBase_Entry code, you will notice the interesting part here:

valuewidget = manage(new class Widget_ValueBase());
valuewidget->inside_cellrenderer();
add(*valuewidget);
valuewidget->show();
//set_can_focus(true);
//set_events(Gdk::KEY_PRESS_MASK | Gdk::KEY_RELEASE_MASK);

I mean the commented set_events(...) code. It looks like this work is not finished here.
Hope this helps.

@nickleus27
Copy link
Contributor

@ice0 @rodolforg thanks guys. This is really helpful information.

@nickleus27
Copy link
Contributor

@ice0 Lol, I have most of the value EventBox's escaping now except when the the EventBox has the Value type of real. Trying to figure that out at the moment.

@rodolforg
Copy link
Contributor Author

Closed by #2455

@rodolforg
Copy link
Contributor Author

Thank you, @nickleus27 ! :)

@nickleus27
Copy link
Contributor

Thank you @rodolforg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants