Skip to content

Fix Keyboard observer not updating SliderValue #19661

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

Conversation

HippoGamus
Copy link
Contributor

Objective

When the CoreSliders on_change is set to None, Keyboard input, like ArrowKeys, does not update the SliderValue.

Solution

Handle the missing case, like it is done for Pointer.

Testing

  • Did you test these changes?
    Yes: core_widgets & core_widgets_observers
    in both examples one has to remove / comment out the setting of CoreSlider::on_change to test the case of on_change being none.

  • Are there any parts that need more testing?
    No not that I am aware of.

  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
    Yes: core_widgets & core_widgets_observers
    in both examples one has to remove / comment out the setting of CoreSlider::on_change to test the case of on_change being none.

  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?
    I tested on linux + wayland. But it is unlikely that it would effect outcomes.

In the case that the slider has no on_change system set, it will now update SliderValue correctly.
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@alice-i-cecile alice-i-cecile requested a review from viridia June 15, 2025 17:44
@alice-i-cecile alice-i-cecile added this to the 0.17 milestone Jun 15, 2025
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-UI Graphical user interfaces, styles, layouts, and widgets D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 15, 2025
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I wonder what CI is complaining about...

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 16, 2025
@alice-i-cecile
Copy link
Member

Okay, that should have fixed CI for you :) I'll grab this in my merge train tomorrow.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 16, 2025
Merged via the queue into bevyengine:main with commit 209866c Jun 16, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants