10000 [Form] Deprecate not configuring the "widget" option of date/time form types by MrYamous · Pull Request #49588 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] Deprecate not configuring the "widget" option of date/time form types #49588

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 19, 2023
Merged

[Form] Deprecate not configuring the "widget" option of date/time form types #49588

merged 2 commits into from
Apr 19, 2023

Conversation

MrYamous
Copy link
Contributor
@MrYamous MrYamous commented Mar 3, 2023 •
edited by nicolas-grekas
Loading
Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets Related to a suggestion made on symfony/symfony-docs#16600
License MIT
Doc PR TODO

This PR proposes to change the default value of the widget option of date/time form types to single_text, so that the corresponding input fields are rendered using native HTML5 widgets by default.

It does so by deprecating not setting the widget option, so that we can change the default in Symfony 7.

@MrYamous MrYamous requested review from xabbuh and yceruto as code owners March 3, 2023 09:59
@carsonbot carsonbot changed the title [WIP] [Form] Add new html5 widget option - DateType / DateTimeType / TimeType [WIP] Add new html5 widget option - DateType / DateTimeType / TimeType Mar 3, 2023
@carsonbot carsonbot added this to the 6.3 milestone Mar 3, 2023
@carsonbot carsonbot changed the title [WIP] Add new html5 widget option - DateType / DateTimeType / TimeType [Form] [WIP] Add new html5 widget option - DateType / DateTimeType / TimeType Mar 3, 2023
@MrYamous MrYamous changed the title [Form] [WIP] Add new html5 widget option - DateType / DateTimeType / TimeType [Form] Add new html5 widget option - DateType / DateTimeType / TimeType Mar 3, 2023
@nicolas-grekas
Copy link
Member
nicolas-grekas commented Mar 6, 2023 •
edited
Loading

Adding a new value for the widget option + planning to deprecate might make it harder for ppl to upgrade if they need to support many versions of Symfony with the same codebase.

Instead, what about deprecating not setting the widget option and making it default to single_text in v7?

@MrYamous
Copy link
Contributor Author
MrYamous commented Mar 6, 2023

Adding a new value for the widget option + planning to deprecate might make it harder for ppl to upgrade if they need to support many versions of Symfony with the same codebase.

Instead, what about deprecating not setting the widget option and making it default to single_text in v7?

I got your point and this sounds good to achieve the same result of simplification.
However, DateTimeType already has a other default value (choice), won't that cause the same problem for supporting many versions ?

@nicolas-grekas
Copy link
Member

We did follow a similar approach in the past. By asking ppl to be explicit in v6, we can then change the default in v7. To support many versions, one should be explicit about the behavior they want in all versions. They can do so immediately.

@MrYamous
Copy link
Contributor Author
MrYamous commented Mar 6, 2023

We did follow a similar approach in the past. By asking ppl to be explicit in v6, we can then change the default in v7. To support many versions, one should be explicit about the behavior they want in all versions. They can do so immediately.

Thanks for explanation
I'll update pr with this approach

@MrYamous MrYamous force-pushed the form/add-widget-html5-option branch from 43635bb to 67893b3 Compare March 11, 2023 23:50
@nicolas-grekas nicolas-grekas force-pushed the form/add-widget-html5-option branch from 67893b3 to 04fac82 Compare April 18, 2023 21:23
@nicolas-grekas nicolas-grekas changed the title [Form] Add new html5 widget option - DateType / DateTimeType / TimeType [Form] Deprecate not configuring the "widget" option of date/time form types Apr 19, 2023
@nicolas-grekas nicolas-grekas force-pushed the form/add-widget-html5-option branch from 04fac82 to a1e3dea Compare April 19, 2023 06:38
nicolas-grekas
nicolas-grekas approved these changes Apr 19, 2023
View reviewed changes
Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a 2nd commit that updates the codebase to always set the widget option, so that we don't trigger the deprecation ourselves.

@nicolas-grekas nicolas-grekas force-pushed the form/add-widget-html5-option branch from a1e3dea to 31c3db9 Compare April 19, 2023 06:46
@nicolas-grekas nicolas-grekas force-pushed the form/add-widget-html5-option branch from 31c3db9 to 138be68 Compare April 19, 2023 11:28
@nicolas-grekas nicolas-grekas force-pushed the form/add-widget-html5-option branch from 138be68 to 1ca33e7 Compare April 19, 2023 11:52
@nicolas-grekas
Copy link
Member

Thank you @MrYamous.

@nicolas-grekas nicolas-grekas merged commit bfd34b3 into symfony:6.3 Apr 19, 2023
@nicolas-grekas nicolas-grekas mentioned this pull request Apr 19, 2023
Closed
@faizanakram99
Copy link
Contributor

This is a breaking change btw, the combination [html5 => false, widget => single_text] is not possible anymore, widget => single_text causes html5 to be true regardless of the given option

@xabbuh
Copy link
Member
xabbuh commented Jul 20, 2023

@faizanakram99 Please open a new issue. Comments on merged pull requests are likely to get lost.

@faizanakram99 faizanakram99 mentioned this pull request Jul 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@xabbuh xabbuh Awaitin 65F0 g requested review from xabbuh xabbuh is a code owner

@yceruto yceruto Awaiting requested review from yceruto yceruto is a code owner

Assignees
No one assigned
Labels
Feature Form Status: Reviewed
Projects
None yet
Milestone
6.3
Development

Successfully merging this pull request may close these issues.
5 participants
@MrYamous @nicolas-grekas @faizanakram99 @xabbuh @carsonbot
0