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

Refactor animation duration config UI #84

Merged
merged 3 commits into from
Jun 26, 2024

Conversation

NamorNiradnug
Copy link
Collaborator

@NamorNiradnug NamorNiradnug commented Jun 24, 2024

Related to #80

@NamorNiradnug
Copy link
Collaborator Author

Requires WayfireWM/wf-config#71

@soreau
Copy link
Member

soreau commented Jun 24, 2024

The PR seems to work ok, but my only nitpick is cosmetic. I would like to retain the order of the combobox of easings as I had it: linear, circle, sigmoid, easeOutElastic. However I'd also like to fix the bug mentioned here, whether it's in another PR or this one.

EDIT:
@ammen99 thankfully came up with this idea, which seems to solve the int-option-to-0-on-Option-destruction.

@NamorNiradnug
Copy link
Collaborator Author

The PR seems to work ok, but my only nitpick is cosmetic. I would like to retain the order of the combobox of easings as I had it: linear, circle, sigmoid, easeOutElastic. However I'd also like to fix the bug mentioned here, whether it's in another PR or this one.

I also noticed the bug back then and even tried to fix it but I didn't find out how. I'll look into that again and share my thoughts.

@NamorNiradnug
Copy link
Collaborator Author

I would like to retain the order of the combobox of easings as I had it: linear, circle, sigmoid, easeOutElastic.

As other easings will be available the order will change however. As I understand your point is that easings should be sorted by "complexity", right?

@soreau
Copy link
Member

soreau commented Jun 24, 2024

As other easings will be available the order will change however. As I understand your point is that easings should be sorted by "complexity", right?

I wanted to use the order listed in wf-config here.

@NamorNiradnug
Copy link
Collaborator Author

I wanted to use the order listed in wf-config here.

Okay, but I think it doesn't really matter for user then? They're sorted in alphabetical order in std::map anyway...

@soreau
Copy link
Member

soreau commented Jun 25, 2024

I wanted to use the order listed in wf-config here.

Okay, but I think it doesn't really matter for user then? They're sorted in alphabetical order in std::map anyway...

It matters to me, I would rather retain the ordering.

Copy link
Member

@soreau soreau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and it actually fixed a bug where the easing was defaulting to linear with the reset-to-defaults button. Thank you for addressing the easing ordering in the combobox. You can you merge this when you're ready. 👍

@NamorNiradnug
Copy link
Collaborator Author

I tested this and it actually fixed a bug where the easing was defaulting to linear with the reset-to-defaults button. Thank you for addressing the easing ordering in the combobox. You can you merge this when you're ready. 👍

Thanks! I think it's ready, but I don't have write access to this repo.

@soreau soreau merged commit 6ab4136 into WayfireWM:master Jun 26, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants