Ref T8362, Add date format preference and respect it in date selection controls
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T8362: Set date time account preferences
- Commits
- Restricted Diffusion Commit
rPd3b7071e70f7: Add date format preference and respect it in date selection controls
Set date format preference in the user settings panels, create new event, select new start date in the correct format.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| src/applications/settings/panel/PhabricatorDateTimeSettingsPanel.php | ||
|---|---|---|
| 81 | Can you add ISO 8601 as well (big endian with - as a separator) | |
| src/applications/settings/panel/PhabricatorDateTimeSettingsPanel.php | ||
|---|---|---|
| 81 | Probably just changing the "Big-Endian" format is enough, as the countries that use that order in practice do not use / as a separator (they use Chinese or Korean characters but ISO format will be familiar to them too). | |
| src/applications/settings/panel/PhabricatorDateTimeSettingsPanel.php | ||
|---|---|---|
| 81 | That's fine with me too. | |
Comment Actions
Big-Endian format should be separated by dashes instead of slashes.
| src/applications/settings/panel/PhabricatorDateTimeSettingsPanel.php | ||
|---|---|---|
| 81 | Yep, this makes sense. | |
Comment Actions
I don't think this will actually work, because there are no meaningful parser changes on the server side, so I don't see how it can distinguish between US and non-US formats after the form is submitted. Specifically, try this:
- Set date format to European (d/m/y).
- Create an event on an ambiguous date (2/3/2015 -- should mean March 2).
I expect the date will be parsed as Feb 3 after you actually save the form, because the PHP code doesn't know that it's supposed to parse things the other way. Does that actually happen, or do things work correctly?
If this does happen, just mangling it before parsing it is a possible approach:
Dates in the m/d/y or d-m-y formats are disambiguated by looking at the separator between the various components: if the separator is a slash (/), then the American m/d/y is assumed; whereas if the separator is a dash (-) or a dot (.), then the European d-m-y format is assumed.
http://php.net/strtotime
strotime() should be the same logic used by new Date().
Otherwise you could try parsing first, then fall back to new Date() to catch ISO 8601. You have to fall back or otherwise end up in new Date() to handle "3 days ago".
| src/applications/settings/panel/PhabricatorDateTimeSettingsPanel.php | ||
|---|---|---|
| 83–84 | Maybe consider calling these "European" (or "Non-US", or "Civilized", or similar, to taste) and "ISO 8601". In particular, "Big-Endian" and "Little-Endian" have a strong significance in programming contexts already (byte order), and these terms feel out of place to me. I theorize other users with C experience may also find the terms unclear. I also think we should make ISO 8601 the default format -- it is unambiguous, while the other two formats are potentially confusing to a lot of users (either all US users or all non-US users). | |
| src/view/form/control/AphrontFormDateControl.php | ||
| 133 | Quotes unnecessary. | |
| 144 | Specifically, consider Y-m-d as a default here instead. | |
| webroot/rsrc/js/core/behavior-fancy-datepicker.js | ||
| 36 | I think we should split on any reasonable separator (any of /, -, ., , at least). We have to choose a separator when rendering or writing a date, but if a user types in 2015.05.23, it's completely unambiguous and there's no reason we can't parse it. | |
| 167–168 | How can we hit this? | |
| 170–172 | I think this is unusual in US dates, at least -- I'd expect 2/28/2015, not 02/28/2015. (But I think it is required by ISO-8601, so it shouldn't just be removed.) | |
| webroot/rsrc/js/core/behavior-fancy-datepicker.js | ||
|---|---|---|
| 167–168 | No, but seems like a reasonable failsafe? | |
| src/view/form/control/AphrontFormDateControlValue.php | ||
|---|---|---|
| 203 | ||
| 210–214 | You can maybe do this a little more cleanly with preg_split(): $parts = preg_split('@[,./:]@', $date); | |
| 220 | I'd expect '-' to be present on this list. | |
| 223–224 | I think this may need to fall back to parsing the unmangled input if this parse of a mangled input fails. For example, suppose I enter a string like:
I'm not sure if that really works (I don't think it does), but you can imagine there likely exist some range of inputs that are valid and contain ,, ., /, or :, but are no longer valid after those characters are replaced with -. In the case above, we'd mangle into something like this:
...which would fail to parse, then could try the original:
...which could succeed. I can't actually come up with any examples of these date inputs (except date inputs which include times, I suppose), so it's possible that none exist. If you think none exist or that waiting for users to report them is fine, that's OK too. | |