-
Notifications
You must be signed in to change notification settings - Fork 23
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
Time-only picker loses date portion of original value #84
Comments
Just curious, why would you bind 2 controls to the same date? On another note, all the plugins I made have both the BTW, nice image.. Tintin 😺 |
I had a project previously where users did not like the combined input on one field, so we separated it out. I think I was using this library and it somewhat worked - but if the user deleted the time input then it would reset. I understand your pain, I added in support for selecting objects instead of values for a selectize wrapper: https://github.com/radical-systems-australia/au-selectize/blob/master/au-selectize.ts and yeah, I get comments in real life that I look like Tintin 😆 |
Ah Selectize was another lib I was looking at, they also have an extension to make something like the bootstrap tagsinput (this one I made but I later found out it doesn't handle array of objects). The Selectize has this "remove_button" plugin
... nice to meet you Tintin haha... which city are u? I went to Sydney 2 times, my best friend moved there. |
Yeah if I was to rewrite the plugins I made, I would drop the 2 properties and only keep 1, mainly the object itself. If a user wish to get just the Your plugin looks quite promising, I'm probably going to use it as well. If you wish I could also add a link to your lib into my lib demo, to give you more visibility. Lately most of my time is spent on my other lib Aurelia-Slickgrid (and it's cousin in Angular). Those are written in TypeScript and having the time, I would rewrite all the Bootstrap Plugins as well in TS... but yeah free time is also rare. Darwin, wow that is quite far up north, isn't that quite warm in that area? I see you're not too far from Indonesia. Ahh I wish I could live in Australia too, seriously I'm Canadian and winters are way too long ⛄️ I'd like to visit more of Australia next time I go, maybe NZ too, seems nice as well. |
I'm starting to feel confident in handling the syncing issue, would be happy to enhance the sync in the datetimepicker when I have the time, maybe as a separate PR. Also I think an I just pushed an update to au-selectize and it's properly syncing now. It was made exceptionally harder by 1. having to sync arrays and 2. listening for mutations as well as assignments, meaning I had to duplicate the logic into a collectionObserver as well as the standard propertyObserver pattern. Should be very stable for use now, I tested it pretty thoroughly 🙂 I'd be happy for you to include a link in your demo, ideally I'll have it set up as an NPM package soon. My colleagues and I really love Aurelia, and want to help the community grow by having these ready to use wrappers. I have a friend who moved to Canada and it looks amazing. I have no solid plans yet, but definitely hope to go there soon. I went to NZ for 3 weeks last year, just hired a car and went around doing hikes and finished the trip with snow boarding. The country is absolutely beautiful and I would highly recommend it! It's pretty much opposite of Darwin, here it is hot, red and flat. There it is green, cool and hilly/mountainous. |
I merged your code and just released a new version of the DateTime Picker, version While I was testing, I found an issue and I'm not sure if I should mention it or not (because it's kinda stupid) but I created myself an infinite loop (or kind of, since I have a If you have any improvement that you want to do to the plugin, please feel free to create PRs. I'm quite open on this and I have to admit that I'm a bit behind with Aurelia since we use Angular at work (not my decision and it sux). These Bootstrap Plugins were the first ones I've made, that is why they are in plain ES6 instead of TypeScript, nowadays I'm all for TS everywhere.
That is actually why I wanted to provide the
I just tried to build the plugin demo and it's actually broken, not sure when I'll have time to look at this. I'll probably need to redo it completely. I remember that Aurelia was breaking often over a year ago, I think they're more stable now and it's probably best to redo a new demo project with all new packages of Aurelia lib to get it up and running. In the meantime, if you have an NPM package, I can add a link in my lib README until I get the demo building again. |
Hopefully this clears it up a bit. So... inputValue would bind to the text box. On update, it will sync with the model, which stores the date object. It can have special logic to not override the date portion of model, if the display format is only time. You can probably add a new valueFormat or something, for users who will be binding to your control using a string, e.g. in my case it's ISO. I understand the changes are not trivial, just an idea for now and something I can probably try a PR for in future. Would aim to not change any binding names to ensure it's backwards compatible. Hope this makes sense. Edit: just to clarify, I changed a few details from what I was explaining before because I hadn't quite thought it through completely |
I'd label this is more of an enhancement, but:
If you bind two controls to the same date, e.g. one control with a date format of
DD/MM/YYYY
and one with a time format ofhh:mm a
, the second control will end up losing the date portion of the original value as it reads it's own strings from the inputvalue
and then syncs with themodel
property.I think the optimal solution here is to have an extra internal property that actually binds with the bootstrap datetimepicker input, e.g.
internalValue
which will then publish any input changes to the bindablemodel
andvalue
properties that are exposed. Using this method we can ensure that the date portion is not lost during the sync process.The same method of identifying if it's only a time value can be taken from the underlying datepicker library (see their hasDate, hasTime and isEnabled functions). This can be used to check for this scenario and ensure that the date portion is not lost.
An example of the issue below in the log statements, you can see as valueChanged triggers an update on model, it loses the date portion and resets to the current date (12th of February in this case).
I'm starting to get an understanding of this wrapper, I might be able to get a PR around this.
The text was updated successfully, but these errors were encountered: