ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.95k stars 13.52k forks source link

ion-range start and end events #17839

Closed JumBay closed 2 years ago

JumBay commented 5 years ago

Feature Request

Ionic version:

[x] 4.x

Describe the Feature Request

As discussed here https://github.com/ionic-team/ionic/issues/17299, it would be nice to have a start event triggered when knob is pressed and an end event triggered when the knob is released like the behavior of ionFocus and ionBlur in ionic v3.

Thanks

biesbjerg commented 5 years ago

I gave it a shot. It's my first pull request-ish, so be gentle ;-) #18250

liamdebeasi commented 5 years ago

Hi there,

Thanks for the feature request. How do the start and end events differ from the focus and blur events? The focus event should fire when the knob is pressed, and the blur is fired when the knob is released.

Thanks!

JumBay commented 5 years ago

That was the behavior in ionic 3 but it has changed in v4 see: https://github.com/ionic-team/ionic/issues/17299#issuecomment-458490016

liamdebeasi commented 5 years ago

Hi everyone,

I spoke with the team, and there are a couple things going on here:

  1. The focus/blur behavior in ion-range isn't quite right. focus should fire when the element comes into focus (tabbing in, clicking, etc). blur should fire when the element loses focus (clicking outside of the range).

  2. We need a way to capture mousedown/mouseup since essentially what we were capturing in v3. I think the PR that was created attempts to resolve this, so I will work with the PR author on cleaning this up.

Thanks!

biesbjerg commented 5 years ago

@liamdebeasi focus/blur seems to work as advertised (like you would expect any input component to behave).

I committed another change that also triggers start/end when changing range using the keyboard. The start-event will always contain the range value before the knob was moved and the end-event will contain the final range value after the knob has been released.

I'm not sure if I'm supposed to squash the commits (or you can do it?) or how - sorry!

liamdebeasi commented 5 years ago

Hi @biesbjerg,

I created a CodePen that shows the ion-range behavior vs the native range input behavior: https://codepen.io/liamdebeasi/pen/eaZyxW. Clicking the track on ion-range fires both the focus and blur events, which is not correct.

My focus/blur comment was in regards to the existing behavior of the ion-range, not in regards to your PR. I will post any feedback for that on the PR page. 🙂

Don't worry about squashing commits, we will squash and merge your PR when it's ready.

Thanks!

biesbjerg commented 5 years ago

@liamdebeasi I agree that the current behavior is not correct regarding focus / blur when clicking the range. I think the issue comes from this line, where the closest knob is getting the focus, triggering focusout on the range. Anyway, this should probably be tracked in a different issue.

What are your thoughts about merging the PR adding start and end events in its current form?

liamdebeasi commented 5 years ago

Hi everyone,

We had been focused on shipping 4.6.0 (and then subsequently fixing some bugs). I haven't forgotten about this, and I hope to loop back to it soon.

Thanks! 🙂

biesbjerg commented 5 years ago

@liamdebeasi I need to get the value when range has stopped moving. I'm using it for seeking in an audio element, and it is not optimal to set position hundreds of times while moving the knob, when all I neeed is the final value. I also tried with various debouncing tricks, which makes it feel sluggish and buggy if audio starts buffering - believe me, I've tried hard to find something that would work for me, but in the end an onDragEnd event is really what I need.

biesbjerg commented 5 years ago

For comparison, material slider has two outputs (change is what I'm after):

image

biesbjerg commented 5 years ago

Note: Native range's onchange emits when knob has been released, not multiple times while dragging the knob, like ion-range currently does.

https://codepen.io/biesbjerg/pen/EqxOoW

liamdebeasi commented 5 years ago

Ah interesting. Can we maybe just have 1 event then rather than start and end events?

biesbjerg commented 5 years ago

change and input events covers most use cases I can think.

Are you suggesting changing the behavior of change and adding input like material?

liamdebeasi commented 5 years ago

My main reservation is that we would be adding 2 new APIs to ion-range to get one desired behavior.

I think ionChange should remain the same since it is in line with how ionChange events behave for other components. Adding an event to know when the knob has been dragged could be useful though.

liamdebeasi commented 5 years ago

It seems like we could emit something here: https://github.com/ionic-team/ionic/blob/master/core/src/components/range/range.tsx#L260

Are you interested in knowing when the knob has been dragged, or just when the range has been updated as a result of a user?

edit: we might want to consider emitting the same event here as well since users could click along the track: https://github.com/ionic-team/ionic/blob/master/core/src/components/range/range.tsx#L238

biesbjerg commented 5 years ago

Personally I'm interested in knowing the value after the range has been dragged and value updated, letting me know the position to seek to in the audio.

Like in my PR: https://github.com/ionic-team/ionic/pull/18250/files#diff-96cc2845be9d5c7024e04f2e1856b65fR246 https://github.com/ionic-team/ionic/pull/18250/files#diff-96cc2845be9d5c7024e04f2e1856b65fR304

Note: I really think changing ionChange's behavior to emit when knob has been released is more in line with how other components work, since technically the value has not changed until that happens (according to how native input[type=range] works).

biesbjerg commented 5 years ago

I do remember I wanted to add TapticEngine feedback (Cordova plugin) to a slider, which requires the following calls:

Tell the taptic engine that a gesture for a selection change is starting. TapticEngine.gestureSelectionStart();

Tell the taptic engine that a selection changed during a gesture. TapticEngine.gestureSelectionChanged();

Tell the taptic engine we are done with a gesture. This needs to be called lest resources are not properly recycled. TapticEngine.gestureSelectionEnd();

In this case you would actually need start, end and the current behavior of change.

I do get that you're trying to minimize API that you have to maintain, but in this case it might hurt the end user by being inflexible.

liamdebeasi commented 5 years ago

One idea we've been playing with in general is updating the ionChange object to include the type of event that triggered it.

So in this case the interface (at least for ion-range) would be something like:

interface ionChangeDetail {
  value: number
  event: Event | null
}

event would take on MouseEvent and similar events. If event is null then we know ionChange was programatic. Would this work for you?

edit: forgot to explain how you'd use this:

You would listen for ionChange and if event !== null then you know the change was done by a user (either through mouse events, keyboard event, etc).

biesbjerg commented 5 years ago

Sounds good to have access to the event that triggered the change to be able to distinguish user changes from programmatic changes.

Would ionChange still fire while dragging the knob or when released? If ionChange triggers when knob is released, not while dragging, I'd be a happy camper :-) But others might need realtime values while dragging is occurs.

onChange(detail: ionChangeDetail) {
  if (detail.event !== null) {
    console.log('Knob was released, seek to position:', detail.value);
  }
}
JumBay commented 5 years ago

I think it will be better for the developer experience, to have the start and end event rather having to check detail.event. It's better to understand what's going on when reading code. Also in my case, I'd like to know when the user has pressed the knob to stop some actions, and restart these actions when it's released.

liamdebeasi commented 5 years ago

@biesbjerg,

I can check in with the team to see what their thoughts on that are. Are you able to use the debounce functionality for this purpose?

biesbjerg commented 5 years ago

Cool :)

No, debounce make it seem sluggish and buggy. I've been beating this horse for a while now. An event on knob release is the only acceptable solution I've found.

biesbjerg commented 5 years ago

I just checked my code, and I rely on both start and end events (it's been a while, currently I'm using a custom built of Ionic to support it).

import { Component, ChangeDetectionStrategy, Input, Output, EventEmitter, ViewChild } from '@angular/core';

import { IonRange } from '@ionic/angular';

@Component({
    selector: 'ip-player-seekbar',
    templateUrl: 'player-seekbar.component.html',
    styleUrls: ['player-seekbar.component.scss'],
    changeDetection: ChangeDetectionStrategy.OnPush
})
export class PlayerSeekbarComponent {

    @ViewChild('seekbar') public seekbar: IonRange;

    @Input() public isWaitingForData: false;
    @Input() public currentTime: number = 0;
    @Input() public duration: number = 0;

    @Output() public seekStart: EventEmitter<number> = new EventEmitter();
    @Output() public seekEnd: EventEmitter<number> = new EventEmitter();

    protected _currentTimeDisplay: number = 0;

    protected isSeeking: boolean = false;

    public get currentTimeDisplay(): number {
        if (this.isSeeking) {
            return this.seekbar.value as number;
        }
        return this.currentTime;
    }

    public set currentTimeDisplay(value: number) {
        this._currentTimeDisplay = value;
    }

    public get remainingTimeDisplay(): number {
        return this.duration - this.currentTimeDisplay;
    }

    public get isDisabled(): boolean {
        return this.isWaitingForData || isNaN(this.duration) || this.duration <= 0;
    }

    public onSeekStart(timeInSeconds: number): void {
        this.isSeeking = true;
        this.seekStart.emit(timeInSeconds);
    }

    public onSeekEnd(timeInSeconds: number): void {
        this.isSeeking = false;
        this.seekEnd.emit(timeInSeconds);
    }

}
<ion-range
    #seekbar
    min="0"
    [max]="duration"
    [disabled]="isDisabled"
    [value]="currentTimeDisplay"
    (ionStart)="onSeekStart($event.target.value)"
    (ionEnd)="onSeekEnd($event.target.value)"></ion-range>

<ion-grid>
    <ion-row>
        <ion-col class="current-time">
            {{ currentTimeDisplay | duration }}
        </ion-col>
        <ion-col class="remaining-time">
            <span *ngIf="remainingTimeDisplay > 0">-</span>{{ remainingTimeDisplay | duration }}
        </ion-col>
    </ion-row>
</ion-grid>

image

liamdebeasi commented 5 years ago

Thanks for the follow up. What if you debounced from your ionChange handler (rather than from within the ion-range component)? So the currentTimeDisplay would update in real time, but you wouldn't actually do the seeking until a certain timeout.

biesbjerg commented 5 years ago

I'm not sure I follow?

Something like:

    public onChange(timeInSeconds: number): void {
        clearInterval(this.disableUpdatesTimer);
        this.isSeeking = true;
        this.disableUpdatesTimer = setTimeout(() => {
            this.isSeeking = false;
            console.log('SEEK END');
        }, 250);

One problem is that it gets triggered by [value]="currentTimeDisplay" binding in template. Another issue is that it wouldn't 'feel' very good seeking, trying to get to a specific position, since actual seeking is delayed by a debounce, compared to doing it at once when knob is released.

liamdebeasi commented 5 years ago

Ok thanks. I'm going to touch base with the team regarding this.

danielehrhardt commented 5 years ago

I think it is really important to have the ionStart and ionEnd Event. So I would really suggest merging this Pull request. Currently, i am not able to use this Component. There is no option to save the Stuff after Change.

danielehrhardt commented 5 years ago

This is a tmp fix. Until the Fix is merged to Ionic.

import { Directive, ElementRef, EventEmitter, Output } from '@angular/core';

@Directive({
  selector: '[range]'
})
export class RangeDirective {
  knobInterval;
  knob: HTMLElement[];

  /**
   * @description event output emitter
   * @property end
   * @type EventEmitter<any>
   * @public
   * @default new EventEmitter<any>()
   */
  @Output('end') public end: EventEmitter<any> = new EventEmitter<any>();

  constructor(
    private el: ElementRef
  ) { }

  /**
   * @description after view initialises, recalcultae height of textarea
   * @method onInput
   * @param nativeElement
   */
  public ngAfterViewInit(): void {
    this.initKnob();
  }

  initKnob(): void {
    this.knobInterval = setInterval(() => {
      const element = this.el.nativeElement;

      if (element.shadowRoot === null) {
        element.attachShadow({ mode: 'open' });
      }
      this.knob = element.shadowRoot.querySelectorAll('.range-knob-handle');

      if (this.knob.length > 0) {
        clearInterval(this.knobInterval);
        this.knob.forEach(knob => {
          knob.ontouchend = () => {
            this.end.emit('change');
          }
        });
      }
    }, 100)
  }

}
biesbjerg commented 5 years ago

@danielehrhardt thank you for the idea!

I've extended it (EDIT: an simplified it!) a bit:

Hopefully we won't have to use this hack for long :-)

import { Directive, ElementRef, EventEmitter, Output, HostListener } from '@angular/core';

import { RangeValue } from '@ionic/core';
import { IonRange } from '@ionic/angular';

@Directive({
    // tslint:disable-next-line:directive-selector
    selector: 'ion-range'
})
export class RangeEventsDirective {
    @Output() public ionStart: EventEmitter<RangeValue> = new EventEmitter();
    @Output() public ionEnd: EventEmitter<RangeValue> = new EventEmitter();

    protected isSliding: boolean = false;

    public constructor(protected elemRef: ElementRef<IonRange>) {}

    @HostListener('mousedown', ['$event'])
    @HostListener('touchstart', ['$event'])
    public onStart(ev: Event): void {
        this.isSliding = true;
        this.ionStart.emit(this.elemRef.nativeElement.value);
        ev.preventDefault();
    }

    @HostListener('mouseup', ['$event'])
    @HostListener('window:mouseup', ['$event'])
    @HostListener('touchend', ['$event'])
    public onEnd(ev: Event): void {
        if (this.isSliding) {
            this.isSliding = false;
            this.ionEnd.emit(this.elemRef.nativeElement.value);
            ev.preventDefault();
        }
    }
}
josephlodero commented 4 years ago

@biesbjerg Hi! where should i put your solution? thanks!

piratenkapitaen commented 4 years ago

@biesbjerg it works, thank you for the fix, I need it for my audio player actually my first directive I implemented :-)

LeonardoMinatiCrispyBacon commented 4 years ago

Hi everyone! I ran into this issue and used mousedown and mouseup events like you guys did on your directive. A problem I'm facing is that the mouseup event isn't triggered if, when dragging the slider, I go outside the area of the range component. Like that the things I do when the value changes cannot be done if the user, sliding the knobs, goes out of the range component.

Are you experimenting this behaviour too?

robsonos commented 4 years ago

Same behaviour here

LeonardoMinatiCrispyBacon commented 4 years ago

Same behaviour here

@robsonos I obtained the behaviour I expected with a little trick. I share my code with you so maybe I can help you with this:

HTML TEMPLATE

<!-- this is the main container of my page -->
<ion-content (mouseup)="mouseUp()">

  <!-- other elements of the page here -->

  <ion-range
    [(ngModel)]="age"
    min="5"
    max="99"
    dual-knobs="true"
    [pin]="true"
    (mousedown)="activateRange()"
  >
    <ion-label slot="start">AGE</ion-label>
  </ion-range>
</ion-content>

JS

@Component({
  selector: ' ... ',
  templateUrl: ' ... ',
  styleUrls: [ ... ]
})
export class CoursesPage implements OnInit, OnDestroy {
  // NOTE THIS VARIABLE
  public rangeActive = false;
  public age = {
    lower: 0,
    upper: 99
  };

  activateRange() {
    this.rangeActive = true;
  }

  mouseUp() {
    if (this.rangeActive) {
      this.rangeActive = false;
      // here you are sure the value is changed and 
      // the user has stopped to drag the range knobs so you can use the values
    }
  }
}

Only in this way I can use a range input that when user drags the knobs going outside the input area still works and triggers the "change" event (it's not the change event, but this trick works like that) only when user releases the knob.

Hope this can help, if you have any question just ask!

biesbjerg commented 4 years ago

@LeonardoMinatiCrispyBacon @robsonos I updated the code to fix the issue. Didn't discover it in my own apps because they're only used on touchscreen devices :-)

LeonardoMinatiCrispyBacon commented 4 years ago

@biesbjerg for sure your solution is the cleanest one! 😃

biesbjerg commented 4 years ago

@LeonardoMinatiCrispyBacon same line of thinking though! ;-)

robsonos commented 4 years ago

@LeonardoMinatiCrispyBacon @biesbjerg thank you guys, works like a charm!

cmer4 commented 4 years ago

Hey all, I was triaging my own code and found this thread after I applied a workaround which is just simple addition to ion-range listeners:

(touchend)="onRangeDragEnd()" (mouseup)="onRangeDragEnd()"

I guess its Angular specific binding here and I "waste" 2 listeners, but other than that - am I missing something?

LeonardoMinatiCrispyBacon commented 4 years ago

Hi @cmer4 ! I think you are missing this point:

the mouseup event isn't triggered if, when dragging the slider, I go outside the area of the range component. Like that the things I do when the value changes cannot be done if the user, sliding the knobs, goes out of the range component

Which is why we implemented the code above 🙂 Hope I got your request right!

cmer4 commented 4 years ago

Ah totally makes sense. So ideally on touchstart/mousedown bound to ion-range we want to dynamically add listener for mouseup/touchend/touchcancel for the entire document/entire ionic page which would warrant user moving the knob too fast and dropping it will still trigger the "dragend" scenario. And then also remove dynamic listener to prevent mem leaks. Cool!

ghenry22 commented 4 years ago

@liamdebeasi this discussion started around a year ago and seems to have dropped off the Ionic team radar.

I have been working on updating an ionic v3 app to v5 and noted that the ionBlur behaviour is changed so rather than firing when I let go of the knob (ie release focus / blur) nothing happens. If i then touch the screen anywhere outside the range element blur fires.

It would be really useful to have blur fire when the knob is released or failing that to have some kind of dragEnd event to catch the value when the user releases the knob.

vinnie667 commented 3 years ago

Has there been any progress with getting this functionality added to ion range?

I wanted to use ion-range inside ion-slides and need to get the (knobReleased) event in order to elegantly deal with the locking and unlocking of swiping the slides.

The blur strategy is clunking as it requires the user to touch off the ion-range, and once again swipe.

pauloanalista commented 3 years ago

Usei a dica de nosso amigo e funcionou pra mim.

Ficou assim!

<ion-col>
        <ion-item lines="none">
          <ion-range 
            min="0" 
            [max]="indexMax" 
            step="60" 
            [value]="indexAtual" 
            [disabled]="jaDeuPlayAlgumaVez==false" 
            [(ngModel)]="indexAtual" 
            (mousedown) = "activateRange()"
            (mouseup) = "mouseUp()">
            <ion-icon size="small" slot="start" name="videocam-outline"></ion-icon>
            <ion-icon slot="end" name="videocam-outline"></ion-icon>
          </ion-range>
        </ion-item>
      </ion-col>

No TS Antes do construtor

public  rangeActive  =  false ; `

Depois do construtor

mouseUp ( )  { 
    if  ( this.rangeActive )  { 
      this.rangeActive = false ; 
      // aqui você tem certeza de que o valor foi alterado e 
      // o usuário parou para arrastar os botões de intervalo para que você possa executar alguma ação
      this.ajustarPosicao();
    } 
  }

  activateRange ( )  { 
    this.rangeActive = true ; 
  }
sundayz commented 3 years ago

Another related issue is that you should be able to control if seeking happens in response to the user tapping on the range. For example on iOS, the Youtube and Spotify apps don't update the seeking bar if you just tap on it. Instead, you have to grab the knob and slide it. Otherwise, it's easy to move the range accidentally.

Start/end events would make it easy to implement this functionality because you could distinguish between the user grabbing the knob and just tapping on the bar.

ApayRus commented 3 years ago

How to use Range for media progress bar? While media is playing value is updating continuously, and it fires onIonChange and seeks video for some milliseconds back:

    const handleSeek = (e: CustomEvent) => {
        console.log(e.detail.value)
        if (playerRef.current) {
            playerRef.current.seek(e.detail.value)
        }
    }

    return (
        <>
            <video controls playsInline ref={mediaRef} style={{ width: '100%' }}>
                <source {...{ src }} />
            </video>
            <IonRange
                min={0}
                max={playerState.duration}
                value={playerState.currentTime}
                onIonChange={handleSeek}
                onIonFocus={() => {
                    console.log('focus')
                }}
                onIonBlur={() => {
                    console.log('blur')
                }}
            />
            <div>{JSON.stringify(playerState)}</div>
        </>
    )
}

For react-native Slider there are 2 additional events: onSlidingStart, onSlidingComplete. How to implement this logic on Ionic?

joeflateau commented 3 years ago

@Aparus I ended up using pointer(down|move|up) events in my audio player. there doesn't seem to be a cleaner way to do it with ion-range 😕

svdoever commented 2 years ago

@Aparus I ended up using pointer(down|move|up) events in my audio player. there doesn't seem to be a cleaner way to do it with ion-range 😕

PointerUp works as long if mouse is over knob, when leaving the control I get a PointerLeave event, but no PointerUp event, and it keeps moving the knob as long as my mouse button is pressed... and keep firing onIonChange events. Difficult to get this right....

stefanhuber commented 2 years ago

I am in the process of developing an audio player app. As the onIonBlur Event is not triggered when the user removes the finger from the knob, I wonder if there is an end event now?

Edit: For determining when the knob dragging ends the events mouseup and touchend seem to work in order to support desktop and smartphone...

ApayRus commented 2 years ago

I solved the issue in that way:

    const handleSeek = (e: CustomEvent): void => {
        const newTime = e.detail.value
        if (Math.abs(newTime - playerState.currentTime) > 0.3) {
            playerHandlers.seek(newTime)
        }
    }
    ...
    <IonRange
        min={start}
        max={end}
        value={currentTime}
        onIonChange={handleSeek}
    />