Time zone issue causing hour and minutes to be off

[gnilk]

Hi,

Having an issue with time-zone (using 2.2.2).
If TZ = Asia/Kuala_Lumpur the time will always be off by 30minutes.
Example (just outline):

const tz = moment.tz.guess();
const t = moment();
console.log(moment.tz(t,tz).format("HH:mm"));
<TimePicker
  timeZone={tz}
  time={t.format("HH:mm")}
  timeFormatter={this.formatTime}
/>

formatTime(timeObject) {
  // time object is off by 30 minutes
}

Set the machine to use Asia/Kuala_Lumpur as the TZ and the picker is off by 30 minutes.
Using 'material' as rendering.

My complete setup of timepicker is:

    <TimePicker
      {...this.props}          
      focused={focused}
      meridiem={meridiem}
      onFocusChange={this.onFocusChange}
      onHourChange={this.onHourChange}
      onMeridiemChange={this.onMeridiemChange}
      onMinuteChange={this.onMinuteChange}
      onTimeChange={this.onTimeChange}
      timeFormatter={this.formatTime}
      timezone={tz}
      showTimezone={false}
      time={moment(this.props.time).format("HH:mm")}
      trigger={this.trigger}
      useTz={true}
      theme='material'
    />

Also when setting showTimezone={true} react bails out.
warning.js?8a56:36 Warning: Failed prop type: Invalid prop children supplied to CSSTransitionGroupChild, expected a ReactNode.
in CSSTransitionGroupChild (created by TransitionGroup)
in TransitionGroup (created by CSSTransitionGroup)
in CSSTransitionGroup (created by Timezone)
in div (created by Timezone)
in Timezone (created by TwentyFourHoursMode)
in div (created by TwentyFourHoursMode)
in TwentyFourHoursMode (created by MaterialTheme)
in div (created by MaterialTheme)
in MaterialTheme (created by TimePicker)
in div (created by OutsideClickHandler)
in OutsideClickHandler (created by TimePicker)
in div (created by TimePicker)
in TimePicker (created by TimePickerWrapper)

Screenshot (correct time is the OS clock in upper right corner):

Br
Fredrik

[gnilk]

Tried to replicate in a standalone environment without luck.
However, there is a difference between FF (57.0.1) and Chrome (62.0.3202.94).

Maybe I am just doing something stupid. Would be good if it was possible to disable any tz handling and just have the picker work with raw hours/minutes leaving it up to caller/user to deal with.

Firefox:

Chrome:

Note: the minutes diff in the screenshot and actual time is due to me writing the comment..

[gnilk]

This most likely not an issue with react-times. I am seeing other very strange things right now.
Sorry...

[gnilk]

Reopening this after some more digging.
Reason: Certain time-zones (esp. in Asia) have offsets depending on the date (i.e. Singapore/Kuala Lumpur) - guessing the time-zone without passing in a fully qualified date/time object can't possibly work (unless I am again missing something).

I believe this is part of the problem (from time.js/getValidTimeData):
const formatTime = moment(1970-01-01 ${validTime}, YYYY-MM-DD ${hourFormat}, 'en');

For Singapore and KualaLumpur this will cause 30min off. As they changed their offset in 1982.
See: moment/moment-timezone#490

A potential fix could be to pass in a reference date which is used instead of 1970-01-01.
IMHO - the best would be to pass in time as a moment object directly. As there is anyway a dependency on moment within the code. OR remove any TZ handling within the library and let the caller handle it.

Br
Fredrik

[ecmadao]

Wow! Thanks a lot for your research! It seems really a problem and can not fixed quickly in react-times it self, but may be it's a good idea to allow module user handle it by themselves. And I think we can invite @carlodicelico and @erin-doyle to discuss this problem -- they did a lot great work to embed timezone function in react-times.
Thanks again for your advice!

[gnilk]

If changed to 'let user deal with it' - you would break backwards compatibility, which is problematic in itself.

One option would be to have a property called 'reference time'. Default this property to currently used 1970-01-01. This would retain backwards compatibility but also allow for users to override. In my case I would simply set it to 'moment()'.