Add support for animation easings #80
Conversation
|
Works well for me. Applied over 0.8 |
src/wcm.cpp
Outdated
| } | ||
| } | ||
|
|
||
| combo_box->set_active(int_type); |
There was a problem hiding this comment.
Consider using set_active_text here (as well as below)
There was a problem hiding this comment.
Any particular reason? Either way, it just looks like it would be the same thing with more code.
|
I tested it as well, seems to be working, thanks! |
src/wcm.cpp
Outdated
| default_value = default_value] | ||
| { | ||
| std::cout << __func__ << std::endl; | ||
| if (easing_type == easing_types[3]) |
There was a problem hiding this comment.
Looks like dangling references: easing_types and easing_type are on stack and captured by reference. Probably easing_types should be made static const. Although capturing it is unnecessary with set_active_text.
UPD: actually I think easing_types list should be provided by wf::animation_description_t.
UPD2: turns out there is such a function: wf::animation::smoothing::get_available_smooth_functions.
There was a problem hiding this comment.
I'll leave this for you to figure out.
| set_value.has_value() ? set_value->easing_name : std::get<std::string>(option-> | ||
| default_value); | ||
| std::string animation_value = std::to_string( | ||
| widget->get_value_as_int()) + "ms " + easing; |
There was a problem hiding this comment.
Maaaybe a better approach would be to add a Option::set_value overload which takes two parameters (int and std::string). It would simplify code here to option->set_save(default_value, default_easing_type) and allow to avoid code duplication (here and below in reset and combo_box->signal_changed() callbacks).
There was a problem hiding this comment.
I'll leave this for you to do.
|
@NamorNiradnug Thanks for the review! I have to fix CI, then I'll rebase this on the CI fix and address your concerns. |
| std::string animation_value = std::to_string( | ||
| default_value) + "ms " + widget->get_active_text(); | ||
| std::cout << animation_value << std::endl; | ||
| option->set_save(animation_value); |
There was a problem hiding this comment.
Actually this set_save call is unnecessary because calling widget->set_active(...) emits signal_changed
There was a problem hiding this comment.
It is necessary to set the combobox widgets to the current value.
|
@NamorNiradnug Thanks for the review. Is there any way you can add commits to this PR, to fix the rest of the things you want to fix? |
|
For what it's worth, there's some bug that keeps setting int options to 0, and also happens on animation easing types. Just adjust an int value and close wcm, it will likely get set to 0. This started happening around the time of the gtkmm port. I would like to fix it because it's quite annoying. |
Fixes #79.