HTML5 Date Fields #6766
HTML5 Date Fields #6766
Conversation
src/Forms/DateField.php
Outdated
| @@ -226,8 +222,20 @@ protected function getFormatter() | |||
| IntlDateFormatter::NONE | |||
| ); | |||
|
|
|||
| // Don't invoke getDateFormat() directly to avoid infinite loop | |||
| if ($this->dateFormat) { | |||
| $isoFormat = 'y-MM-dd'; | |||
There was a problem hiding this comment.
Heh I was looking for a ICU constant for this but couldn't find it - good call!
src/Forms/DateField.php
Outdated
| * | ||
| * @var bool | ||
| */ | ||
| protected $html5 = false; |
There was a problem hiding this comment.
Shall we set this to true by default? In most cases it'll be more useful than a textfield right?
There was a problem hiding this comment.
Hmmm yeah I guess for most frontend implementations, it'll be a better default (opt-out of HTML5 rather than opt-in). For the CMS, it'll definitely be a better default, since we have a polyfill there. And it would save devs remembering that when writing getCMSFields(). I'll change it, and update the changelog
src/Forms/DateField.php
Outdated
| $clientView->onBeforeRender(); | ||
| if ($this->getHTML5()) { | ||
| // Browsers expect ISO 8601 dates, localisation is handled on the client | ||
| $this->setDateFormat('y-MM-dd'); |
There was a problem hiding this comment.
Instead of modifying the state on render, can we move this logic to getDateFormat()? I realise we violate this in many places, but we should probably make fields immutable during render.
There was a problem hiding this comment.
Good idea. I want to perform it as late as possible, but overriding the getter behaviour is more internally consistent.
src/Forms/TimeField.php
Outdated
| * | ||
| * @var bool | ||
| */ | ||
| protected $html5 = false; |
There was a problem hiding this comment.
As above, should this be true by default?
There was a problem hiding this comment.
Agreed, it's a better default - I'll change.
src/Forms/TimeField.php
Outdated
| @@ -140,8 +165,20 @@ protected function getFormatter() | |||
| $this->getTimezone() | |||
| ); | |||
|
|
|||
| // Don't invoke getDateFormat() directly to avoid infinite loop | |||
| if ($this->timeFormat) { | |||
| $isoFormat = 'HH:mm:ss'; | |||
src/Forms/TimeField.php
Outdated
|
|
||
| // Try to parse time without seconds, since that's a valid HTML5 submission format | ||
| // See https://html.spec.whatwg.org/multipage/infrastructure.html#times | ||
| if ($timestamp === false && $this->setHTML5(true)) { |
There was a problem hiding this comment.
Did you mean to use $this->getHTML5() as a condition? This method shouldn't mutate the object.
|
|
||
| New `TimeField` methods replace `getConfig()` / `setConfig()` | ||
|
|
||
| * `getTimeFormat()` / `setTimeFormat()` | ||
| * `getLocale()` / `setLocale()` | ||
| * `getClientConfig()` has been removed (in favour of `setHTML5()`) | ||
|
|
||
| #### <a name="overview-template-removed"></a>Template and Form Removed API |
There was a problem hiding this comment.
Need to add notes on the removal of MemberDatetimeOptionsetField to this section.
There was a problem hiding this comment.
Maybe mention DateField_View_JQuery too here.
| @@ -81,9 +80,6 @@ class Member extends DataObject implements TemplateGlobalProvider | |||
| 'Locale' => 'Varchar(6)', | |||
| // handled in registerFailedLogin(), only used if $lock_out_after_incorrect_logins is set | |||
| 'FailedLoginCount' => 'Int', | |||
| // In ISO format | |||
| 'DateFormat' => 'Varchar(30)', | |||
src/Security/Member.php
Outdated
| return $format; | ||
| } | ||
| return $this->getDefaultDateFormat(); | ||
| $format = $this->getDefaultDateFormat(); |
There was a problem hiding this comment.
We can probably just merge getDefaultDateFormat and getDateFormat
src/Security/Member.php
Outdated
| return $timeFormat; | ||
| } | ||
| return $this->getDefaultTimeFormat(); | ||
| $format = $this->getDefaultTimeFormat(); |
There was a problem hiding this comment.
As above, we may as well merge these. The other place getDefault* methods were called are gone.
chillu
force-pushed
the
pulls/4.0/6626-remove-jquery-datepicker
branch
from
d64c236
to
3b94d14
Compare
|
@chillu your tests seem broken. Do they pass locally? |
See #6626