twinssbc / Ionic2-Calendar

A calendar component based on Ionic framework
https://ionic-calendar-demo.stackblitz.io
MIT License
388 stars 197 forks source link

MonthViewCalendar: onRangeChanged was fired twice in queryMode=remote #133

Closed longgt closed 5 years ago

longgt commented 7 years ago

It seems to be onRangeChanged event was fired twice in queryMode = remote. During view initialization, slide to next/previous slide, onRangeChanged will be fired twice. As logic which fetch event data is implemented in onRangeChanged, data will be also fetched twice. During time selected, onRangeChanged is only fired one time.

If I make some changes to logic in onRangeChanged by

onRangeChanged(event) {
    if(this.range.startTime != event.startTime || this.range.endTime != event.endTime) {
        //Do fetch event source
    }
    this.range.startTime= event.startTime;
    this.range.endTime = event.endTime;
} 

Although event source was fetched successfully, it will not be displayed in month view.

Please let me know onRangeChanged was fired twice is "by design" or this is a bug? How to fetch data only one time on onRangeChanged event during view initialization, slide to next/previous slide?

Thank you!

P/S: Stack traces during onRangeChanged event fired [The first time stack trace]

HomePage.onRangeChanged (home.ts:100)
CalendarService.rangeChanged (calendar.service.js:34)
MonthViewComponent.refreshView (monthview.js:272)
MonthViewComponent.move (monthview.js:78)
MonthViewComponent.onSlideChanged (monthview.js:68)
(anonymous) (MonthViewComponent.html:3)

[The second time stack trace]

HomePage.onRangeChanged (home.ts:100)
CalendarService.rangeChanged (calendar.service.js:34)
MonthViewComponent.refreshView (monthview.js:272)
(anonymous) (monthview.js:25)
CalendarService.setCurrentDate (calendar.service.js:14)
set (calendar.js:46)
longgt commented 7 years ago

Workaround for me now

In calendar.service.ts, change logic to trigger to next subscriber only if this._currentDate is different from parameter val value.


    setCurrentDate(val: Date, fromParent: boolean = false) {
        let changed = true;

        if(this._currentDate && val && moment(this._currentDate).format('YYYYMMDD') == moment(val).format('YYYYMMDD')) {
            changed = false;
        }
        this._currentDate = val;
        //Trigger to next subscriber only if this._currentDate != val
        if(changed) {
            if (fromParent) {
                this.currentDateChangedFromParent.next(val);
            } else {
                this.currentDateChangedFromChildren.next(val);
            }
        }
    }

In HomePage.ts

onRangeChanged(event) {
    if(this.range.startTime != event.startTime || this.range.endTime != event.endTime) {
        //Do fetch event source
    }
    this.range.startTime= event.startTime;
    this.range.endTime = event.endTime;
}
twinssbc commented 7 years ago

@longgt I can confirm the onRangeChanged event is triggered only once during the view initialization. Once you slide to previous/next month, it will also be triggered, but only once. Sounds like you want to slide to the previous/next slide during view initialization? You just need to set the initial calendar.currentDate to a date in previous/next month.

longgt commented 7 years ago

@twinssbc Thank you for your answer.

I've just confirmed with latest version (0.3.7) and when sliding to prev/next month, onRangeChanged was triggered twice, not once.

I'm using Ionic 3.4.0 and here is how to reproduce.

  1. Start new Ionic project with sidemenu template
  2. Install the latest version of ionic2-calendar (0.3.7)
  3. Source In HomePage.ts

    calendar = {
        mode: 'month',
        currentDate: new Date(),
        queryMode: 'remote'
    };
    
    onRangeChanged(ev) {
        console.log(ev);
    }

    In HomePage.html

    <calendar [eventSource]="eventSource"
        [calendarMode]="calendar.mode"
        [currentDate]="calendar.currentDate"
        [queryMode]="calendar.queryMode"
        [monthviewEventDetailTemplate]="monthviewEventDetailTemplate"
        (onCurrentDateChanged)="onCurrentDateChanged($event)"
        (onRangeChanged)="onRangeChanged($event)"
        (onEventSelected)="onEventSelected($event)"
        (onTitleChanged)="onViewTitleChanged($event)"
        (onTimeSelected)="onTimeSelected($event)">
    </calendar>

    Just like your answer, during view initialization, I confirmed that onRangeChanged was triggered only once. But when sliding to prev/next month, onRangeChanged was triggered twice.

The first time onRangeChanged was triggered is in MonthViewComponent.onSlideChanged function and the second is in view re-render time. In Developer Tools console, the log from onRangeChanged will be displayed twice.

twinssbc commented 7 years ago

@longgt It makes sense that the first trigger is caused by the onSlideChanged when you swipe the view. The second trigger which is caused by the setCurrentDate only happens the calendar.currentDate is set from outside of the calendar. Is there any logic in your code that is changing calendar.currentDate?

longgt commented 7 years ago

@twinssbc No, calendar.currentDate is set to current date in the view initialization and unchanged after that. I think when swiping to prev/next view, as autoSelect is set to true (default value), the value of calendar.currentDate in HomePage.ts will be changed and when the view is re-rendered, because CalendarComponent thinks it changed, onRangeChanged event is triggered.

autoSelect
If set to true, the current calendar date will be auto selected when calendar is loaded 
or swiped in the month view.
Default value: true
twinssbc commented 7 years ago

@longgt onRangeChanged is called in refreshView method. There are two types of currentDateChanged events in the calendar code.

        this.currentDateChangedFromParent$ = this.currentDateChangedFromParent.asObservable();
        this.currentDateChangedFromChildren$ = this.currentDateChangedFromChildren.asObservable();

refreshView is only called in the currentDateChangedFromParent event which is triggered from outside of calendar. The currentDate change inside the calendar won't cause this.

        this.currentDateChangedFromParentSubscription = this.calendarService.currentDateChangedFromParent$.subscribe(currentDate => {
            this.refreshView();
        });