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

bug: range ionInput event behavior inconsistent with documentation #29619

Closed
3 tasks done
bodinaren opened this issue Jun 13, 2024 · 2 comments · Fixed by #30293
Closed
3 tasks done

bug: range ionInput event behavior inconsistent with documentation #29619

bodinaren opened this issue Jun 13, 2024 · 2 comments · Fixed by #30293
Labels
type: bug a confirmed bug report

Comments

@bodinaren
Copy link

bodinaren commented Jun 13, 2024

Prerequisites

Ionic Framework Version

v7.x, v8.x

Current Behavior

The onIonInput event behavior isn't consistent with the documentation for the event. This is the current description:

The ionInput event is fired for <ion-range> elements when the value is modified. Unlike ionChange, ionInput is fired continuously while the user is dragging the knob.

The actual behavior is that the event is triggered whenever the knob is moved a single pixel (after an initial "sticky" behavior) instead of it triggering whenever the actual value is modified, causing a lot of events being triggered for absolutely no reason. The second part "is fired continously" is ambivalent and could match either expectation.

Expected Behavior

I would expect the event to trigger only when the value is modified rather than when the knob is moved.

Steps to Reproduce

  1. Open the reproduction Stackblitz link.
  2. Open the browsers developer console.
  3. Drag the slider a short distance until the first ionInput event is triggered, then keep moving the knob a short distance and see how the event continues to be logged even without the value changing.

Code Reproduction URL

https://stackblitz.com/edit/umlmf6?file=src%2Fmain.tsx

Ionic Info

Ionic:

Ionic CLI : 7.2.0.

Utility:

cordova-res : not installed globally
native-run : not installed globally

System:

NodeJS : v20.9.0
npm : 10.1.0
OS : Windows 10 (actual version 11)

Additional Information

As I see it there's 3 possible solutions:

  1. Only trigger the event when the value has changed from it's previous value as the documentation says.
  2. Same as 1 but only when snaps: true.
  3. Update the documentation to not specify when the value is modified but rather something like when the knob is moved.

I believe 1 is the preferable solution, but it might be considered a breaking change and so 2 is a reasonable compromise as there's no practical nor visual updates between value-changes when snaps: true making it a non-breaking change.

3 is by far the least preferable solution in my opinion as this issue is not only a documentation error but the current implementation also carries an unnecessary performance impact (again, at least when snaps: true).

@ionitron-bot ionitron-bot bot added the triage label Jun 13, 2024
@brandyscarney brandyscarney added type: bug a confirmed bug report and removed triage labels Sep 3, 2024
@brandyscarney
Copy link
Member

Thank you for the issue! I agree that the best solution would be to match the behavior of native input ranges and only emit the event when the value changes from the previous one. I’ve marked this as a bug, which adds it to our internal issue tracker for further work.

github-merge-queue bot pushed a commit that referenced this issue Mar 24, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Issue number: resolves #29619 

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

The `ionInput` emits for range even when the value hasn't changed. This
does not match our documentation. It should only emit when the value
changes (and continuously while the user is dragging the knob).

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Moved the emitter to the value watch function, to determine if the
value has changed.
- Added a test
-

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

How to test:
1. Navigate to the [range basic HTML
file](https://github.com/ionic-team/ionic-framework/blob/8ed08fcba59c9fde5c6d08d721fd1d00642de4f9/core/src/components/range/test/basic/index.html#L77)
2. Add the following script
```
const ionicRanges = document.querySelectorAll('ion-range');

ionicRanges.forEach(range => {  
  range.addEventListener('ionInput', function(ev) {
    console.log('ionInput', ev.currentTarget.value);
  });
});
```
3. Navigate to the [range test
page](http://localhost:3333/src/components/range/test/basic)
4. Open the console
5. Move the single knob range (let go when you're done)
6. Verify that the value is shown in the console
7. Tap as close to the middle of the knob. The goal is to tap it without
the value moving.
8. Verify that the value does not show in the console

---------

Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
@thetaPC
Copy link
Contributor

thetaPC commented Mar 24, 2025

Thank for submitting the issue! This has been resolved via PR #30293 and will be available in an upcoming release of Ionic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants