8000 [Form] Change datetime to datetime-local for HTML5 datetime input by pierredup · Pull Request #24401 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] Change datetime to datetime-local for HTML5 datetime input #24401

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 1 commit into from
Mar 20, 2018
Merged

[Form] Change datetime to datetime-local for HTML5 datetime input #24401

merged 1 commit into from
Mar 20, 2018

Conversation

pierredup
Copy link
Contributor
Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? Unknown
Deprecations? no
Tests pass? yes
Fixed tickets #24307
License MIT
Doc PR N/A

Change the input type from datetime to datetime-local for HTML5 date input (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/datetime)

@nicolas-grekas
Copy link
Member

Is browser support good enough?
https://caniuse.com/#search=datetime-local

@nicolas-grekas nicolas-grekas added this to the 2.7 milestone Oct 2, 2017
@ogizanagi
Copy link
Contributor

Anyway, isn't it a BC break? JS apps may rely on type="datetime". Not sure it is worth it.

@stof
Copy link
Member
stof commented Oct 2, 2017

type="datetime" is supported by exactly 0 browser out there, meaning checking the type DOM property gives you text due to the type fallback.
And the datetime type is removed from the HTML spec.

However, the HTML5_FORMAT constant has the timezone included in the format. So it should be updated for datetime-local to remove the timezone part.

@xabbuh
Copy link
Member
xabbuh commented Oct 4, 2017

Is this enough? I mean is the format in which data is sent by browsers the same as it was meant to with the old datetime type?

@Tobion
Copy link
Contributor
Tobion commented Oct 4, 2017

From my short test, it seems Chrome submits the datetime-local without a timezone. So @stof would be right that \Symfony\Component\Form\Extension\Core\Type\DateTimeType::HTML5_FORMAT needs to be changed.

Tobion
Tobion suggested changes Oct 4, 2017
View reviewed changes
Copy link
Contributor
@Tobion Tobion left a comment

Choose a reason for hiding this comment

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

html5 constant and description

@pierredup
Copy link
Contributor Author

Would it not be considered a BC break to change the value of a constant?

@simPod
Copy link
Contributor
simPod commented Dec 31, 2017

What's the status of this? Can I help?

@nicolas-grekas
Copy link
Member

@pierredup let's do the change as requested by @Tobion /@stof

@pierredup pierredup force-pushed the datetime-input branch 2 times, most recently from f2b7874 to 31c9c1e Compare February 7, 2018 07:39
@nicolas-grekas
Copy link
Member

@Tobion aren't we ready here?

@fabpot
Copy link
Member
fabpot commented Mar 20, 2018

Thank you @pierredup.

@fabpot fabpot merged commit 802c67c into symfony:2.7 Mar 20, 2018
fabpot added a commit that referenced this pull request Mar 20, 2018
@fabpot
bug #24401 [Form] Change datetime to datetime-local for HTML5 datetim…
0877bf6 
…e input (pierredup)

This PR was merged into the 2.7 branch.

Discussion
----------

[Form] Change datetime to datetime-local for HTML5 datetime input

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | Unknown
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #24307
| License       | MIT
| Doc PR        | N/A

Change the input type from datetime to datetime-local for HTML5 date input (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/datetime)

Commits
-------

802c67c Change datetime input to datetime-local
This was referenced Apr 2, 2018
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

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

@Tobion Tobion Tobion approved these changes

Assignees
No one assigned
Labels
Bug Form Status: Reviewed
Projects
None yet
Milestone
2.7
Development

Successfully merging this pull request may close these issues.
9 participants
@pierredup @nicolas-grekas @ogizanagi @stof @xabbuh @Tobion @simPod @fabpot @carsonbot
0