apptreesoftware / pikaday-dart

package:js bindings for Pikaday by David Bushnell.
BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

Options.onSelect gets a Date parameter (not DateTime) #1

Closed simon-void closed 7 years ago

simon-void commented 7 years ago

hi, i'm the author of the pikaday_datepicker_angular2 component (https://pub.dartlang.org/packages/pikaday_datepicker_angular2). Two weeks ago i switched to your wrapper of pikaday (mine was still using dart:js, but why reinvent the wheel). Now i got an Issue and debugging led me to your code. Basically your PikadayOptions.onSelect gets a callback for a DateTime. I used:

_options.onSelect = allowInterop((DateTime day) {
  //event-emit the day
  _dayChange.add(day);
}

This works in Dartium but fails once transpiled to JS, because - supprisingly - day is not of type DateTime then, but of type Date (the js-Type your DateJS class is wrapping).

So how do i get a DateTime instance? I tried your helper-function

var sinceEpochMillies = getPikadayMillisecondsSinceEpoch(_pikaday);
var day = new DateTime.fromMillisecondsSinceEpoch(sinceEpochMillies);

even with allowInterop-wrapper

var sinceEpochMillies = allowInterop(getPikadayMillisecondsSinceEpoch)(_pikaday);
var day = new DateTime.fromMillisecondsSinceEpoch(sinceEpochMillies);

but get a TypeError: self.getPikadayMillisecondsSinceEpoch is not a function[Läs mer]. (I didn't forget to import 'package:pikaday/pikaday_dart_helpers.dart';)

So i'm not sure how to aquire the selected DateTime instance?

johnpryan commented 7 years ago

Hey - I'm glad someone else is using it :)

I think you might have just forgotton to import the pikaday_dart_helpers.js file:

<script type="text/javascript" src="/packages/pikaday/pikaday_dart_helpers.js"></script>
simon-void commented 7 years ago

Hi, i couldn't import it from packages but when i copied the pikaday_dart_helpers.js file into my web/jsLibs folder, it worked. I would be happier though if there was a solution without this import, because i want to minimize the things the users of my component have to import in order to use it. I guess the issue is, that package:js doesn't support DateTime yet. (your bug report) Couldn't you expose the year, month, day parameters of your DateJS class? That seems to be the type of the date that should be DateTime. I'm currently using your getPikadayMillisecondsSinceEpoch-helper

_options.onSelect = allowInterop((dateTimeOrDate) {
  var day = dateTimeOrDate is DateTime
          ? dateTimeOrDate
          : new DateTime.fromMillisecondsSinceEpoch(
              getPikadayMillisecondsSinceEpoch(_pikaday));
  _dayChange.add(day);
}

but i could equally well (and without the extra js-file-import), write something like

_options.onSelect = allowInterop((dateTimeOrDate) {
  var day = null;
  if (dateTimeOrDate is DateTime) {
    day = dateTimeOrDate;
  } else {
    var dateJs = dateTimeOrDate as DateJS;       
    day = new DateTime(dateJs.year, dateJs.month, dateJs.day);
  }
  _dayChange.add(day);
}
johnpryan commented 7 years ago

I couldn't import it from packages but when i copied the pikaday_dart_helpers.js file into my web/jsLibs folder, it worked.

That's odd - it should be right next to the pikaday.js file under packages/pikaday

Couldn't you expose the year, month, day parameters of your DateJS class? That seems to be the type of the date that should be DateTime

That's not a bad idea; if you can get this working i'd be happy to merge a PR.

As far as the dateTimeOrDate approach; it's an unfortunate workaround due to Dartium supporting DateTime conversions but not Chrome; here's what I am doing:

    var date = pikaday.getDate();

    if (date is! DateTime) {
      // This is a workaround until
      // https://github.com/dart-lang/sdk/issues/28329 is resolved.
      var ms = getPikadayMillisecondsSinceEpoch(pikaday);
      date = new DateTime.fromMillisecondsSinceEpoch(ms);
    }
simon-void commented 7 years ago

In any case, version 2.1.4. of my package pikaday_datepicker_angular2 is now out incorporating all your suggestions. Thanks for the help!

johnpryan commented 7 years ago

package:js is nice; consider taking a look at dart_js_facade_gen for how to generate: https://www.npmjs.com/package/dart_js_facade_gen

essentially I run this after making a change to the .d.ts file:

dart_js_facade_gen lib/pikaday.d.ts > lib/pikaday.dart

Awesome - let me know if you find any other issues