[Form] Deprecate not honoring with_seconds => false for HTML5 datetime inputs in DateTimeType #49158
Conversation
097820f to
622ca4f
Compare
| @@ -54,7 +67,7 @@ public function transform(mixed $dateTime): string | |||
| $dateTime = $dateTime->setTimezone(new \DateTimeZone($this->outputTimezone)); | |||
| } | |||
|
|
|||
| return $dateTime->format(self::HTML5_FORMAT); | |||
| return $dateTime->format($this->withSeconds ? self::HTML5_FORMAT : self::HTML5_WITHOUT_SECONDS_FORMAT); | |||
| } | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
Reverse transform already works without seconds. We can't "remove" the seconds from a \DateTime instance so there's nothing else to do
622ca4f to
5f80f84
Compare
...ny/Component/Form/Extension/Core/DataTransformer/DateTimeToHtml5LocalDateTimeTransformer.php
Outdated
Show resolved
Hide resolved
| $options['view_timezone'] | ||
| )); | ||
| // Keep only the code inside this if in Symfony 7.0 | ||
| if ($options['with_seconds']) { |
There was a problem hiding this comment.
I am not really sure I understand these two branches. Effectively this means that starting with 7.0 setting with_seconds to false won't be supported anymore for HTML5 widgets, right?
There was a problem hiding this comment.
In 7.0, with_seconds =>
8000
false won't trigger a deprecation anymore and will therefore be "allowed" and will be honored (seconds won't be displayed in the widget).
There was a problem hiding this comment.
What I am concerned about is that with the proposed solution we do not offer any upgrade path to opt-in to the new behaviour with with_seconds being false on 7.0 without getting a deprecation in 6.3/6.4.
There was a problem hiding this comment.
Indeed, because with_seconds => false currently does not work and we can't change the behavior without forcing everyone to change it to true first.
5f80f84 to
44de252
Compare
44de252 to
60638da
Compare
…e inputs in DateTimeType
60638da to
9e57977
Compare
|
Closing in favor of #50059 |
…ess "with_seconds" is explicitly set (fancyweb) This PR was merged into the 6.3 branch. Discussion ---------- [Form] Don't render seconds for HTML5 date pickers unless "with_seconds" is explicitly set | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | yes | Deprecations? | no | Tickets | Fix #48076 | License | MIT | Doc PR | - Alternative to #49158 (commit credits due to `@fancyweb`) I don't think we need a deprecation for this change. To me, it's a bugfix. But merging it on 6.3 might be safer so let's do it as a new feature. Commits ------- 93008ee [Form] Don't render seconds for HTML5 date pickers unless "with_seconds" is explicitly set
The
with_secondsoption defaults to false. It is honored when the form doesn't have data (and when the browser doesn't display the seconds). But it's not honored when the form has data becauseDateTimeToHtml5LocalDateTimeTransformerformat the\DateTimeInterfacetoY-m-d\\TH:i:sunconditionally.In this PR, we add a new argument to
DateTimeToHtml5LocalDateTimeTransformerto allow formatting to a datetime string without seconds (Y-m-d\\TH:i).Moreover, we deprecate not setting
with_seconds => trueon all forms using HTML5 datetime inputs since they rely on that behavior (and it's the default value of thewith_secondsoption).In Symfony 7.0, we can honor the option (by forwarding its value to the transformer) because every project should be using
with_seconds => truebefore upgrading.I don't really know if forcing everyone to add/set the option is the best strategy but I did not find another one.