Skip to content
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

U4-7301 - Extend datepicker options #984

Closed
wants to merge 19 commits into from

Conversation

bjarnef
Copy link
Contributor

@bjarnef bjarnef commented Dec 29, 2015

PR for http://issues.umbraco.org/issue/U4-7301

It extend the datepicker with use of more properties and also make it possible to use it just as a timepicker.

There is a few issues that need to be fixed or considered.

  • The "Publish at" and "Unpublish at" use the datetime picker, but it should always be possible to specify the full date for these - make create an instance for these that can't be deleted a "lock" some properties? .. at the moment I think you also get some errors if you e.g. delete the "Label" datatype instance.
  • When saving datetime values in minimum and maximum properties, e.g. "2015-12-28T00:00:00" and "2015-12-30T00:00:00" in the string properties, they are changed to "28/12/2015 00:00:00" and "30/12/2015 00:00:00" ... but I can't directy pass these as string into Date object in javascript, because it is invalid in English format.
  • With disabled dates you can't pick values lower than (minimum date) or greather than (maximum date) - but you can enter dates "outside" the limit in the input (but when saving you seem to get "invalid date" error). If entering a date lower than minimum date (if specified) can we set it to the minimum date? and similar for maximum date? ..
  • Maybe install it via bower. The current version used is 3.1.3, but there seems to be a 3.1.4 version https://www.versioneye.com/javascript/eonasdan:eonasdan-bootstrap-datetimepicker/3.1.4 with some minor changes.
  • Notice that pickDate, pickTime, useSeconds and useMinutes are set based on the format, e.g. HH:mm will set pickDate=false, pickTime=true, useSeconds=false, useMinutes=true

localizationService.localize("content_invalidDate").then(function (value) {
$scope.errorMsg.invalid = value;
});
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error messages wasn't localized .. maybe we can use a similar approach for the other property editors?

<div ng-repeat="day in weekdays">
<label><input type="checkbox" name="weekdays" checklist-model="selectedDays" checklist-value="day.id" /> {{day.name}}</label>
</div>
</div>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the weekdays prevalues doesn't need to be specific datetime picker?
Probably other might want to use this as well? .. I store an array of the selected days, where Sunday=0, Monday=1, etc... but they can of course just link to this view in datepicker folder.

@warrenbuckley
Copy link
Contributor

@bjarnef are you waiting on @Shazwazza to clarify some of this before it's ready to be reviewed or do you wish for me to review it in it's current state?

@bjarnef
Copy link
Contributor Author

bjarnef commented Apr 6, 2016

@warrenbuckley you are welcome to review, but there is a few issues with setting min and max date. It seems when I enter a date format in PE config, it re-format it on save? maybe moment.js does something there?

Let me know if I should change more on this.

@warrenbuckley
Copy link
Contributor

@bjarnef Yes before I can review this properly, I suggest that you try to complete this & ensure you have tested the different scenarios. Moment.js is great for dealing & working with dates & I recommend using it if you need to or need help with date formats & locales etc. This is shipped int the core of Umbraco.

With dealing with some stuff with date pickers in forms, I know there can be a lot to test. I would recommend you ensure that the following works:

  • Just time picker
  • Just date picker
  • Test combination of both
  • Testing the localisation of these to the user as per the backoffice user's language & locale
  • Test that this does not break Publish at or Unpublish
  • Tripple check we do not use this DatePicker property editor anywhere else

Note Locale: However ensure the correct date format is saved/sent to the DB. We are only interested in changing how the date looks when picked

@bjarnef
Copy link
Contributor Author

bjarnef commented Apr 6, 2016

Okay, will check this.
However it seems when I enter a raw date format for minDate & maxDate in a textstring prevalue editor it is re-formatted after save.
What is the best way to save min and max dates?

@bjarnef
Copy link
Contributor Author

bjarnef commented Apr 6, 2016

Furthermore for publish at and unpublish at I think they need a separate instance of datepicker as they always should use a full datetime picker.

@warrenbuckley
Copy link
Contributor

@bjarnef is that the problem that is causing you a problem to finish this PR?
Care to attach a screenshot please so I understand what the problem is fully.

@bjarnef
Copy link
Contributor Author

bjarnef commented Apr 6, 2016

A little as in think I think moment.js want the date format in English, e.g. I think it was re-formatted in Danish date format from prevalue input.. It is a while ago I last had a look at is, so need to take a look at it again :)
I'll add some screenshots when I review it again.

@warrenbuckley
Copy link
Contributor

OK look forward to seeing your updates, I will try to help where I can to help achieve your goal with this :)

…v-v7-U4-7301

Conflicts:
	src/Umbraco.Web.UI.Client/src/views/propertyeditors/datepicker/datepicker.html
@bjarnef
Copy link
Contributor Author

bjarnef commented Apr 7, 2016

@warrenbuckley I have merged latest changed from dev-7 into this branch.

So we have these properties for datetime property editor.

image
image

The date format YYYY-MM-DD HH:mm:ss decides whether it should be a time picker, date picker or datetime picker.

If I enter these dates for minimum and maximum dates:
image

then after save they somehow is changed to this format (danish datetime format):
image
Maybe it somewhere use my local culture, because I am logged in backoffice as administrator using English (en-GB).
image

Do you know how we handle these correctly? .. I think moment.js understand datetime format in other locales, but it then the culture/language/locale passed in - otherwise it might use a wrong date or can't convert it to a datetime if e.g. it think 31 is the month instead of the day.

http://momentjs.com/docs/#/parsing/string-formats/

moment("29-06-1995", ["MM-DD-YYYY", "DD-MM", "DD-MM-YYYY"]); // uses the last format
moment("05-06-1995", ["MM-DD-YYYY", "DD-MM-YYYY"]);          // uses the first format
moment("29-06-1995", ["MM-DD-YYYY", "DD-MM-YYYY"], 'fr');       // uses 'fr' locale
moment("29-06-1995", ["MM-DD-YYYY", "DD-MM-YYYY"], true);       // uses strict parsing
moment("05-06-1995", ["MM-DD-YYYY", "DD-MM-YYYY"], 'fr', true); // uses 'fr' locale and strict parsing

We might be able to use something like this:

$scope.model.config.minDate = moment($scope.model.config.minDate), ["YYYY-MM-DD HH:mm:ss"], $scope.model.config.language);
                $scope.model.config.maxDate = moment($scope.model.config.maxDate), ["YYYY-MM-DD HH:mm:ss"], $scope.model.config.language);

but I am not sure which format of the dates in config to expect.

@bjarnef
Copy link
Contributor Author

bjarnef commented Apr 7, 2016

Some other things to consider/test.

  • Client side/server side validation of dates lower than minimum date (if set) and maximum date (if set).
  • If one enter a date lower than minimum date then change/set it to minimum date and if one enter a date greater than maximum date then change/set it to maximum date.
  • At the moment it use current date/time as default when focusing in input, but as mentioned before a date (e.g. current date) might be lower than minimum date or greather than maximum date.
  • Ensure changes in this datetime picker doesn't conflict with instances of "publish at" and "unpublish at" datetime pickers.

@Shazwazza
Copy link
Contributor

Since I can enter any date time doesn't like HH MM mm yyyy, will your code take this into account to determine if time is required?

@Shazwazza
Copy link
Contributor

(sorry typo above... Writing on my phone but you get the idea)

@bjarnef
Copy link
Contributor Author

bjarnef commented Apr 7, 2016

My idea was that it always save a datetime and it set pickDate and pickTime depending on the format contains Y, M, D, H, m and s.

So I think there are three scenarios:

2016-04-01T12:12:12 (date and time)
2016-04-01T00:00:00 (date, but also date/time if you choose at midnight)
0000-00-00T12:12:12 (only time)

I am not sure if this is a bug in the plugin, but if I set these dates manually in the script:

$scope.model.config.minDate = "2016-04-01T00:00:00";
$scope.model.config.maxDate = "2016-04-30T12:00:00";

and use moment.js

$scope.model.config.minDate = moment($scope.model.config.minDate);
$scope.model.config.maxDate = moment($scope.model.config.maxDate);

The I can enter the datetime for 2016-04-01 20:00:00 in the input, but when using the "spinners" in timepicker I can only decrease to 1 (for hour) not down to 0 (although I just can enter 00 for hour in input or choose or picking "00" in the extra view when clicking hour between the spinners.

image

Near maximum date I can "spin" 1 second over maximum date.
image

It is only an issue when minimum and/or maximum dates are set (or maybe also and issue in the current datetime picker in core when I think it by default use minimum date = moment({ y: 1900 } and maximum date = moment().add(100, 'y')

@bjarnef bjarnef changed the title Extend datepicker options U4-7301 - Extend datepicker options Nov 22, 2016
@nul800sebastiaan
Copy link
Member

Hi Bjarne,

It's a shame that this has stalled for almost two years now and plenty has happened since then to the datepicker. I think changing it so it supports more options still has a ton of merit but I would encourage you to start fresh instead of us trying to puzzle all of this back together again.

I'll close this PR but it will be good to refer to if you decide to try this one again. We're working on a much better process of dealing with PRs so it won't stall like this any more.

Many thanks for the effort and cooperation and hopefully we'll see a new PR related to this!

@bjarnef
Copy link
Contributor Author

bjarnef commented Mar 14, 2018

Hi Sebastiaan

Yes, it has been a while since I was working on this :)
I think I was stuck at entering a raw datetime value in a textstring prevalue editor, where the datetime was re-formatted (I think to the locale of the current backoffice user), so the format was no longer in a format like 2016-04-01T00:00:00

Maybe we should have a datepicker prevalue editor, which property editors can use similar to the property editor. This could be used for setting minimum and maximum dates in the datetime property editor, but also for other custom property editors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants