Skip to content

[Form] Add tel and color types #22679

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

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,16 @@
{{ block('button_widget') }}
{%- endblock reset_widget -%}

{%- block tel_widget -%}
{%- set type = type|default('tel') -%}
{{ block('form_widget_simple') }}
{%- endblock tel_widget -%}

{%- block color_widget -%}
{%- set type = type|default('color') -%}
{{ block('form_widget_simple') }}
{%- endblock color_widget -%}

{# Labels #}

{%- block form_label -%}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<?php echo $view['form']->block($form, 'form_widget_simple', array('type' => isset($type) ? $type : 'color'));
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<?php echo $view['form']->block($form, 'form_widget_simple', array('type' => isset($type) ? $type : 'tel'));
2 changes: 2 additions & 0 deletions src/Symfony/Component/Form/Extension/Core/CoreExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ protected function loadTypes()
new Type\SubmitType(),
new Type\ResetType(),
new Type\CurrencyType(),
new Type\TelType(),
new Type\ColorType(),
);
}
}
33 changes: 33 additions & 0 deletions src/Symfony/Component/Form/Extension/Core/Type/ColorType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Form\Extension\Core\Type;

use Symfony\Component\Form\AbstractType;

class ColorType extends AbstractType
{
/**
* {@inheritdoc}
*/
public function getParent()
{
return TextType::class;
}

/**
* {@inheritdoc}
*/
public function getBlockPrefix()
{
return 'color';
}
}
33 changes: 33 additions & 0 deletions src/Symfony/Component/Form/Extension/Core/Type/TelType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Form\Extension\Core\Type;

use Symfony\Component\Form\AbstractType;

class TelType extends AbstractType
Copy link
Contributor

Choose a reason for hiding this comment

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

i definitely prefer TelephoneType here 🤔

Copy link
Author

@apetitpa apetitpa May 9, 2017

Choose a reason for hiding this comment

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

I was quite unsure about what name to use, so I chose the one that matches the "tel" HTML5 input name but I have to admit it is less clear

Copy link
Contributor

Choose a reason for hiding this comment

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

i think fully qualified is preferred :)

Copy link
Author

Choose a reason for hiding this comment

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

I'll rename the class then thank you !

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Still no consensus about how the class should be named ? @fabpot @nicolas-grekas @xabbuh

Copy link
Author

@apetitpa apetitpa Sep 21, 2017

Choose a reason for hiding this comment

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

Hello, I'd like to finish this PR, how should I name the class ? TelType, TelephoneType, PhoneType etc ? @symfony/deciders @fabpot

Copy link
Contributor

Choose a reason for hiding this comment

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

either way no blocker for me :) i understand we just want type="tel" one way or another 👍 (my reasoning still stands though(.

Copy link
Contributor

@jvasseur jvasseur Sep 21, 2017

Choose a reason for hiding this comment

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

I think we should go with types that match the html types because that's we we tend to do instead of trying to name them based of the semantic of the field (CheckboxType, TextareaType, RangeType are good examples of this).

Copy link
Member

@nicolas-grekas nicolas-grekas Oct 2, 2017

Choose a reason for hiding this comment

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

@ro0NL I think your reasoning was wrong here when stating this (about "tel"):

we should not name our class after an implementation detail

HTML5 is exactly the opposite to an implementation detail: it's a public interface, a standard, the tip of the iceberg. So we should definitely not seek to be more clever than the ppl who wrote it (we're not ;) ).

{
/**
* {@inheritdoc}
*/
public function getParent()
{
return TextType::class;
}

/**
* {@inheritdoc}
*/
public function getBlockPrefix()
{
return 'tel';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2468,4 +2468,34 @@ public function testButtonAttributeNameRepeatedIfTrue()
// foo="foo"
$this->assertSame('<button type="button" id="button" name="button" foo="foo" class="btn-default btn">[trans]Button[/trans]</button>', $html);
}

public function testTel()
{
$tel = '0102030405';
$form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\TelType', $tel);

$this->assertWidgetMatchesXpath($form->createView(), array('attr' => array('class' => 'my&class')),
'/input
[@type="tel"]
[@name="name"]
[@class="my&class form-control"]
[@value="0102030405"]
'
);
}

public function testColor()
{
$color = '#0000ff';
$form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\ColorType', $color);

$this->assertWidgetMatchesXpath($form->createView(), array('attr' => array('class' => 'my&class')),
'/input
[@type="color"]
[@name="name"]
[@class="my&class form-control"]
[@value="#0000ff"]
'
);
}
}
28 changes: 28 additions & 0 deletions src/Symfony/Component/Form/Tests/AbstractLayoutTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2463,4 +2463,32 @@ public function testAttributesNotTranslatedWhenTranslationDomainIsFalse()
$this->assertMatchesXpath($html, '/form//input[@title="Foo"]');
$this->assertMatchesXpath($html, '/form//input[@placeholder="Bar"]');
}

public function testTel()
{
$tel = '0102030405';
$form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\TelType', $tel);

$this->assertWidgetMatchesXpath($form->createView(), array(),
'/input
[@type="tel"]
[@name="name"]
[@value="0102030405"]
'
);
}

public function testColor()
{
$color = '#0000ff';
$form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\ColorType', $color);

$this->assertWidgetMatchesXpath($form->createView(), array(),
'/input
[@type="color"]
[@name="name"]
[@value="#0000ff"]
'
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"Symfony\\Component\\Form\\Extension\\Core\\Type\\CheckboxType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\ChoiceType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\CollectionType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\ColorType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\CountryType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\CurrencyType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\DateIntervalType",
Expand All @@ -27,6 +28,7 @@
"Symfony\\Component\\Form\\Extension\\Core\\Type\\ResetType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\SearchType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\SubmitType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\TelType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\TextType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\TextareaType",
"Symfony\\Component\\Form\\Extension\\Core\\Type\\TimeType",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
----------------------------------------------------------------

BirthdayType, ButtonType, CheckboxType, ChoiceType, CollectionType
CountryType, CurrencyType, DateIntervalType, DateTimeType, DateType
EmailType, FileType, FormType, HiddenType, IntegerType
LanguageType, LocaleType, MoneyType, NumberType, PasswordType
PercentType, RadioType, RangeType, RepeatedType, ResetType
SearchType, SubmitType, TextType, TextareaType, TimeType
TimezoneType, UrlType
ColorType, CountryType, CurrencyType, DateIntervalType, DateTimeType
DateType, EmailType, FileType, FormType, HiddenType
IntegerType, LanguageType, LocaleType, MoneyType, NumberType
PasswordType, PercentType, RadioType, RangeType, RepeatedType
ResetType, SearchType, SubmitType, TelType, TextType
TextareaType, TimeType, TimezoneType, UrlType

Service form types
------------------
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Form/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@
"phpunit/phpunit": "<4.8.35|<5.4.3,>=5.0",
"symfony/dependency-injection": "<3.3",
"symfony/doctrine-bridge": "<2.7",
"symfony/framework-bundle": "<2.7",
"symfony/framework-bundle": "<3.4",
"symfony/http-kernel": "<3.3.5",
"symfony/twig-bridge": "<2.7"
"symfony/twig-bridge": "<3.4"
},
"suggest": {
"symfony/validator": "For form validation.",
Expand Down