EZP-30263 : User date format don't apply to date fields in content item update and preview #909
Conversation
pawbuj
requested review from
webhdx,
adamwojs,
mikadamczyk,
Nattfarinn and
ViniTou
pawbuj
changed the title
| @@ -80,7 +76,7 @@ | |||
| {% block ezdate_field %} | |||
| {% spaceless %} | |||
| {% if not ez_is_field_empty( content, field ) %} | |||
| {% set field_value = field.value.date|localizeddate( 'short', 'none', parameters.locale, 'UTC' ) %} | |||
| {% set field_value = field.value.date|ez_full_date %} | |||
There was a problem hiding this comment.
UTC has to be forced here to avoid timezone issues.
| {% else %} | ||
| {% set field_value = field.value.time|localizeddate( 'none', 'short', parameters.locale, 'UTC' ) %} | ||
| {% endif %} | ||
| {% set field_value = field.value.time|ez_full_time %} |
There was a problem hiding this comment.
Same as above - UTC has to be forced.
SylvainGuittard
changed the title
| {% endif %} | ||
| {% endspaceless %} | ||
| {% endblock %} | ||
|
|
||
| {% block ezdate_field %} | ||
| {% spaceless %} | ||
| {% if not ez_is_field_empty( content, field ) %} | ||
| {% set field_value = field.value.date|ez_full_date %} | ||
| {% set field_value = field.value.date|ez_full_date('UTC') %} |
There was a problem hiding this comment.
We will be displaying ezdate in UTC, regardless of user time zone from settings?
There was a problem hiding this comment.
@tischsoic same as in the prev implementation, I do not change this behaviour
There was a problem hiding this comment.
Now it will be inconsistent with date picked in edit mode considering last changes: #893
Or maybe we are going to revert those changes?
ping @SylvainGuittard
There was a problem hiding this comment.
It cannot be UTC. The editor will be confused.
There was a problem hiding this comment.
I agree with @webhdx especially that this is a fieldtype value not a regular UI date or time
There was a problem hiding this comment.
@lserwatka @webhdx I understand your point. But the date picker depends on the user preferences and the time zone can be different than UTC. So the date in preview might be different from the date the user selected. Or did I miss something?
There was a problem hiding this comment.
Let's discuss this in private on Monday.
There was a problem hiding this comment.
I was thinking about it and it actully makes sense for me to have eztime and ezdate always in UTC.
For timezone caclulations to work predictably you basically have to have time + date and only ezdatetime has both.
There was a problem hiding this comment.
After internal discussion, ezdate and eztime should use UTC.
Thanks guys.
| const userPreferedFullTimeFormat = eZ.adminUiConfig.dateFormat.full_time; | ||
| const userPreferedShortDateTimeFormat = eZ.adminUiConfig.dateFormat.short_datetime; | ||
| const userPreferedShortDateFormat = eZ.adminUiConfig.dateFormat.short_date; | ||
| const userPreferedShortTimeFormat = eZ.adminUiConfig.dateFormat.short_time; |
There was a problem hiding this comment.
all keys in adminUiConfig should be camelCase
| const formatFullDateTime = (date, timezone = null, format = userPreferedFullDateTimeFormat) => { | ||
| return formatDate(date, format, timezone); | ||
| }; | ||
| const formatFullDate = (date, timezone = null, format = userPreferedFullDateFormat) => { |
There was a problem hiding this comment.
What is the reason for having this method? It's not used anywhere, there are more methods like this here.
There was a problem hiding this comment.
Those function will be added to helpers.timezone in 2.0 , currently it is not possible because of a name conflict with deprecated function formatShortDate
There was a problem hiding this comment.
So this is the dead code? No point of having this in LTS 2.5.
| const formatDateWithTimezone = (date, timezone = userPreferedTimezone, format = userPreferedFullDateFormat) => { | ||
| return formatDate(convertDateToTimezone(date, timezone), format); | ||
| const formatFullDateTime = (date, timezone = null, format = userPreferedFullDateTimeFormat) => { | ||
| return formatDate(date, format, timezone); |
There was a problem hiding this comment.
Can we also unify the parameters order? Here is date, format, timezone but in the wrapper function is date, timezone, format
| formatShortDateWithTimezone, | ||
| formatFullDateTime, | ||
| formatShortDateTime, | ||
| formatDate: deprecatedFormatDate, |
There was a problem hiding this comment.
What about other bundles which are using those deprecated methods? We should correct them to not use the deprecated one.
There was a problem hiding this comment.
There was a problem hiding this comment.
I think there is also one in data-based-publisher.
There was a problem hiding this comment.
| @@ -63,7 +63,7 @@ | |||
| const datetimeConfig = { | |||
| enableTime: true, | |||
| time_24hr: true, | |||
| formatDate: (date) => new Date(date).toLocaleString(), | |||
| formatDate: (date) => eZ.helpers.timezone.formatFullDateTime(date), | |||
There was a problem hiding this comment.
This one will be converted but other in flatpickr are not. Intentional?
There was a problem hiding this comment.
On the second thought, all should be converted to user timezone, this is displayed date.
|
Tool version : PHP CS Fixer 2.7.1 Sandy Pool by Fabien Potencier and Dariusz Ruminski diff --git a/src/lib/UI/Config/Provider/FieldType/RichText/AlloyEditor.php b/src/lib/UI/Config/Provider/FieldType/RichText/AlloyEditor.php
index 9f0e1b1..44e1117 100644
--- a/src/lib/UI/Config/Provider/FieldType/RichText/AlloyEditor.php
+++ b/src/lib/UI/Config/Provider/FieldType/RichText/AlloyEditor.php
@@ -1,5 +1,9 @@
<?php
+/**
+ * @copyright Copyright (C) eZ Systems AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
declare(strict_types=1);
namespace EzSystems\EzPlatformAdminUi\UI\Config\Provider\FieldType\RichText;
@@ -14,7 +18,8 @@ class AlloyEditor implements ProviderInterface
/**
* @param array $alloyEditorConfiguration
*/
- public function __construct(array $alloyEditorConfiguration) {
+ public function __construct(array $alloyEditorConfiguration)
+ {
$this->alloyEditorConfiguration = $alloyEditorConfiguration;
}
@@ -24,7 +29,7 @@ class AlloyEditor implements ProviderInterface
public function getConfig(): array
{
return [
- 'extraPlugins' => $this->getExtraPlugins()
+ 'extraPlugins' => $this->getExtraPlugins(),
];
}
|
…em update and preview (ezsystems#909) * EZP-30263 : User date format don't apply to date fields in content item update and preview * forces UTC on ezdate and eztime * formats ezdate, eztime, ezdatetime according to user preferences * fixes cs * default timezone form user preferences
Adds into ez_full_datetime, ez_full_date, ez_full_time in ezdate_fields.
Warning, this PR can be merged only if ezsystems/ezplatform-user#26 is merged
Checklist:
$ composer fix-cs)