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

Datetime formats #4539

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

gsteinert
Copy link

Description of changes

A few changes to DatetimeType to address #3160

Major Changes

format option removed. In its place dateFormat, timeFormat and tzFormat created. This allows the formats used in DatetimeType to be shared with DatetimeField. Now the display format specified in the three new options is used for validation and the admin UI fields.

parseFormat option behaviour changed.

Previously, a format specified in parseFormat would be the only acceptable format for validation purposes. With an incorrectly specified parseFormat option, you could end up in a situation where a date chosen from the datepicker wouldn't validate.

Now, any format specified in parseFormat is added to the list of acceptable formats. This list already includes the default list and the format specified in dateFormat, timeFormat and tzFormat. parseFormat now only needs to be set if you want to accept additional formats not already in the list - in many cases it can be omitted, even with a custom display format.

Other changes

The format specified in dateFormat, timeFormat and tzFormat is used to format the admin UI fields.

The placeholder in the time field in the admin UI now reflects timeFormat.

Documentation updated

Related issues (if any)

Testing

  • Please confirm npm run test-all ran successfully.
    No new failures introduced. 7 pre-existing failures in unrelated areas remain.

…stonejs#3160

Properties in DatetimeType were not being carried through to
DatetimeField. Now resolved.

format property replaced by dateFormat and timeFormat properties.

Requires properly set parseFormat property (inclusing time zone format
(Z) to function correctly. We should be able to calculate this based on
the dateFormat and timeFormat properties and pass through form the Type
to the Field.
…g instead of hardcoding in DatetimeField.

Updated validation for options to reflect change from format to dateFormat, timeFormat and tzFormat.
dateFormat, timeFormat and tzFormat specified.

This removes the requirement for a parseFormat option to be specified.

The parseFormat option is still available, and if supplied is appended
along with the calculated format to the default list.
…3160

Placeholder for time field now displays correct format.
the same array being shared by all Datetime field instances.

Fixed test 'should throw when format is not a string'. Now fails if
exception is not thrown, rather than timing out.
Altered test for validation of the default format when a parseFormat
is specified.

Previously, if a parseFormat is specified, this became the only
acceptable format, even if a different display format is defined.
If the display format differs from the parse format, the field will
not validate as populated from the database.

Now, if a parseFormat (or formats, in an array) is specified it will
be added to the list of acceptable formats (the default array plus
the display format).
@gautamsi
Copy link
Member

why add breaking changes where you could have the format option alongside other options. you can check if both options are provided and throw error.

not removing format option would have been better for people already using it.

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

Successfully merging this pull request may close these issues.

3 participants