Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

3.0 form input type datetime #2898

Merged
merged 2 commits into from Feb 27, 2014

Conversation

Projects
None yet
3 participants
Owner

lorenzo commented Feb 25, 2014

I removed a bunch of tests that were duplicating the tests for a simple datetime, also removed the useless label 'for' appending of the input subtype (day, month or year) as it actually did not make the label clickable.

@markstory markstory commented on the diff Feb 26, 2014

src/View/Helper/FormHelper.php
@@ -1100,43 +1100,6 @@ protected function _extractOption($name, $options, $default = null) {
*/
protected function _inputLabel($fieldName, $label, $options) {
$labelAttributes = [];
- $idKey = null;
- if ($options['type'] === 'date' || $options['type'] === 'datetime') {
@markstory

markstory Feb 26, 2014

Owner

I was thinking that input() would be where we automatically generate id attribute values so that we can preserve the clicky labels. With that said the id generation can be much simpler now as there is less magic to workaround.

@lorenzo

lorenzo Feb 26, 2014

Owner

That's how it works right now for all the other inputs, but I don't think having a for associated to any of the datetime inputs make sense and not sure it would be possible even.

@markstory

markstory Feb 26, 2014

Owner

FormHelper used to use the ID of the first select box. Which is why this code is a bit gnarly.

@lorenzo

lorenzo Feb 26, 2014

Owner

Not sure how to keep this feature, then. You cannot know what the first input is without parsing the template yourself and not sure how to pass the id to only the first select.

@markstory

markstory Feb 26, 2014

Owner

I'm ok with not keeping the auto-label ID personally. If it is important we can revisit it later.

@ADmad

ADmad Feb 26, 2014

Member

I would like to keep the 'for' of label target first select of the date time selects, if it's not too much hassle to achieve. We can skip it for later though.

@markstory

markstory Feb 26, 2014

Owner

The tricky part is knowing which input will be first. Knowing that requires parsing/pattern matching the template, and inspecting the options. Perhaps we can do that as a separate pull request?

@lorenzo

lorenzo Feb 26, 2014

Owner

One way you can do it is by nesting the inputs into the label, that way it
will alway select the first one

On Wed, Feb 26, 2014 at 3:14 PM, Mark Story notifications@github.comwrote:

In src/View/Helper/FormHelper.php:

@@ -1100,43 +1100,6 @@ protected function _extractOption($name, $options, $default = null) {
*/
protected function _inputLabel($fieldName, $label, $options) {
$labelAttributes = [];

  •   $idKey = null;
    
  •   if ($options['type'] === 'date' || $options['type'] === 'datetime') {
    

The tricky part is knowing which input will be first. Knowing that
requires parsing/pattern matching the template, and inspecting the options.
Perhaps we can do that as a separate pull request?

Reply to this email directly or view it on GitHubhttps://github.com/cakephp/cakephp/pull/2898/files#r10080404
.

@ADmad

ADmad Feb 26, 2014

Member

Hmm having inputs nested inside label it not very flexible for styling, so I am unsure we should do that by default.

Though it seems foundation 5 seems to be doing that for all inputs so can't be too bad.

@markstory markstory added this to the 3.0.0 milestone Feb 26, 2014

markstory added a commit that referenced this pull request Feb 27, 2014

@markstory markstory merged commit 2c6d6ee into 3.0 Feb 27, 2014

1 check passed

default The Travis CI build passed
Details

@markstory markstory deleted the 3.0-form-input branch Feb 27, 2014

Owner

markstory commented Feb 27, 2014

Merging, we can continue to discuss how to best handle label for attributes on datetimes as a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment