Skip to content
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.

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

[Reference][Form Types] Document "with_minutes" time/datetime option #3429

Merged
merged 2 commits into from
Jan 7, 2014
Merged

[Reference][Form Types] Document "with_minutes" time/datetime option #3429

merged 2 commits into from
Jan 7, 2014

Conversation

bicpi
Copy link
Contributor

@bicpi bicpi commented Jan 6, 2014

Q A
Doc fix? yes
New docs? yes
Applies to 2.3+
Fixed tickets #3410 (task time > with_minutes)

The datetime field also has a with_minutes option.

Using the single_text widget type might not work as expected using with_minutes=false if the browser supports HTML5's time input type. Most browsers seem to only support hh:mm or hh:mm:ss formats for the value attribute (resetting the value to --:-- if using an hour value only). This might not be the case for every browser because I couldn't find anything in the HTML5 spec that says a partial time cannot be an hour only. I've added a caution box for this.

input to capture minutes.

.. versionadded:: 2.2
The ``with_minutes`` option was introduced in Symfony 2.2.
Copy link
Member

Choose a reason for hiding this comment

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

this should be placed before the with_minutes headline

Copy link
Member

Choose a reason for hiding this comment

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

We are not really consistent in that but in most cases the versionadded directive is placed below the type line. By the way, do we need it here at all since 2.2 reached EOM?

Copy link
Member

Choose a reason for hiding this comment

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

I 2.3, 2.2 didn't reach EOM. It should be removed in 2.4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, http://symfony.com/doc/current/contributing/documentation/overview.html#documenting-new-features-or-behavior-changes seems to be quite clear about all that. The version note should precede the new/changed feature (currently only valid for one out of six note in the form reference) and version notes are only removed from new doc branches and then only for Symfony version that reached EOL, not EOM. But then you may have to add a version note for a EOL release if you are working on an existing branch like 2.3 and add a missing documention ;-)

@xabbuh
Copy link
Member

xabbuh commented Jan 6, 2014

👍

@weaverryan
Copy link
Member

Thanks Philipp.

weaverryan added a commit that referenced this pull request Jan 7, 2014
…time option (bicpi)

This PR was merged into the 2.3 branch.

Discussion
----------

[Reference][Form Types] Document "with_minutes" time/datetime option

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | yes
| Applies to    | 2.3+
| Fixed tickets | #3410 (task `time > with_minutes`)

The `datetime` field also has a `with_minutes` option.

Using the `single_text` widget type might not work as expected using `with_minutes=false` if the browser supports HTML5's `time` input type. Most browsers seem to only support `hh:mm` or `hh:mm:ss` formats for the value attribute (resetting the value to `--:--` if using an hour value only). This might not be the case for every browser because I couldn't find anything in the HTML5 spec that says a partial time cannot be an hour only. I've added a `caution` box for this.

Commits
-------

1e88b9d Fix "versionadded" position
8cfb850 [Reference][Form Types] Document "with_minutes" time/datetime option
@weaverryan weaverryan merged commit 1e88b9d into symfony:2.3 Jan 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants