X Tutup
Skip to content

SpinBox: Fix custom_arrow_step by snapping it to step#108196

Merged
Repiteo merged 1 commit intogodotengine:masterfrom
beicause:spinbox-custom-arrow-step-snap
Jul 17, 2025
Merged

SpinBox: Fix custom_arrow_step by snapping it to step#108196
Repiteo merged 1 commit intogodotengine:masterfrom
beicause:spinbox-custom-arrow-step-snap

Conversation

@beicause
Copy link
Contributor

@beicause beicause commented Jul 2, 2025

Fixes #108191.

The documentation is correct before #107302. But now when the two steps are inconsistent, step takes precedence.

@beicause beicause requested review from a team as code owners July 2, 2025 10:39
@AThousandShips AThousandShips added this to the 4.5 milestone Jul 2, 2025
@shahriarlabib000
Copy link
Contributor

does this break compatibility?

@shahriarlabib000
Copy link
Contributor

#97561 seems to be Reproducible again

@beicause beicause marked this pull request as draft July 2, 2025 14:03
@beicause beicause force-pushed the spinbox-custom-arrow-step-snap branch from b63d7be to 28e9cfa Compare July 2, 2025 14:30
@beicause
Copy link
Contributor Author

beicause commented Jul 2, 2025

#97561 seems to be Reproducible again

I tested again and found that #107302 did not fix #97561(sorry for that wrong comment). It seems to occur only when the step is 0. Pushed the change to resolve it.

@shahriarlabib000
Copy link
Contributor

bool use_custom_arrow_step should be removed as it is not being used anymore since #107302

@beicause beicause force-pushed the spinbox-custom-arrow-step-snap branch from 28e9cfa to 15ffd92 Compare July 2, 2025 15:11
@beicause beicause marked this pull request as ready for review July 2, 2025 15:23
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected. Code and documentation look good to me.

@Calinou
Copy link
Member

Calinou commented Jul 4, 2025

One issue I noticed though is that if step is 0, then step will become custom_arrow_step, which breaks manual input. This is probably unintended.

This is effectively what #107302 aimed to fix, but this PR reverts it to fix another issue. We probably need to think of a better solution in the long run.

@shahriarlabib000
Copy link
Contributor

So from now on custom_arrow_step needs to be a multiple of step to function properly…
Maybe this should be documented

@beicause
Copy link
Contributor Author

beicause commented Jul 5, 2025

So from now on custom_arrow_step needs to be a multiple of step to function properly… Maybe this should be documented

Not exactly, it works the same as a multiple of step, but doesn't need to be a multiple of step.

@beicause
Copy link
Contributor Author

beicause commented Jul 6, 2025

That seems like a bug in #97573. This behavior was never intended 😅.
behavior should have remained the same as 4.3

I do see some spinbox does not round by arrow step by default (e.g. in KDE Plasma), but there seem to be some are.
Perhaps we can add a property to control whether the arrows round the value.

@shahriarlabib000
Copy link
Contributor

shahriarlabib000 commented Jul 6, 2025

seems like a good idea. But it can probably wait till 4.6

@beicause
Copy link
Contributor Author

beicause commented Jul 6, 2025

Opened a separate PR #108335 for that.

@Repiteo Repiteo merged commit 8702b38 into godotengine:master Jul 17, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jul 17, 2025

Thanks!

@beicause beicause deleted the spinbox-custom-arrow-step-snap branch July 18, 2025 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Custom arrow step doesn't work properly for SpinBox

5 participants

X Tutup