Skip to content

[Form] choices ids are malformed #1919

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

Closed
wants to merge 3 commits into from
Closed

[Form] choices ids are malformed #1919

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
97f667b
dasheized field name
brikou Aug 7, 2011
0eecc42
fixed
brikou Aug 9, 2011
9af78fe
fixed duplicates ids
brikou Aug 9, 2011
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
47 changes: 41 additions & 6 deletions src/Symfony/Component/Form/Extension/Core/Type/FieldType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,37 @@ public function buildForm(FormBuilder $builder, array $options)
*/
public function buildView(FormView $view, FormInterface $form)
{
$name = $form->getName();
$types = array_map(function($type) {
Copy link
Member

Choose a reason for hiding this comment

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

I would have kept the for loop as done in the original code (it's faster).

Copy link
Contributor

Choose a reason for hiding this comment

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

what about array_map('strval', $form->getTypes()); is that also slow? (would simplify the for or foreach loop)

return $type->getName();
}, $form->getTypes());

if (in_array('radio', $types) || in_array('checkbox', $types)) {

$slugs = array();

foreach (array_keys($view->getParent()->get('choices')) as $value) {
$slug = $this->slugify($value);
$slugs[$slug][] = $value;
}

$slug = $this->slugify($form->getName());

// trick to make the slug unique
if (count($slugs[$slug]) > 1) {
$index = array_search($form->getName(), $slugs[$slug]);
$slug.= ' '.$index;
}

} else {
$slug = $this->slugify($form->getName());
}

// this is used to keep the 1st underscore for _token but i guess it
// would be better to name it "-token" to keep things consistent
if ($slug{0} === '-') {
$slug{0} = '_';
}
$name = $slug;

if ($view->hasParent()) {
$parentId = $view->getParent()->get('id');
Expand All @@ -82,11 +112,6 @@ public function buildView(FormView $view, FormInterface $form)
$fullName = $name;
}

$types = array();
foreach ($form->getTypes() as $type) {
$types[] = $type->getName();
}

$view
->set('form', $view)
->set('id', $id)
Expand Down Expand Up @@ -176,4 +201,14 @@ private function humanize($text)
{
return ucfirst(strtolower(str_replace('_', ' ', $text)));
}

private function slugify($str, $delimiter = '-')
{
$clean = iconv('UTF-8', 'ASCII//TRANSLIT', $str);
Copy link
Member

Choose a reason for hiding this comment

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

Several problems here:

Copy link
Contributor

Choose a reason for hiding this comment

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

What about using the Inflector from Lithium? i believe it does all this.

Copy link
Member

Choose a reason for hiding this comment

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

Inflection is very different from slugification and does not solve the same problem. I've just had a quick look at the Inflector class in Lithium and the transliteration part is just plain wrong.

$clean = preg_replace("/[^a-zA-Z0-9\/_|+ -]/", '', $clean);
$clean = strtolower(trim($clean, '-'));
$clean = preg_replace("/[\/_|+ -]+/", $delimiter, $clean);
Copy link
Member

Choose a reason for hiding this comment

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

why removing the underscores ? they are valid AFAIK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just to keep a single delimiter

Copy link
Member

Choose a reason for hiding this comment

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

but this is what force you to use a hack for the CSRF token field. And the form framework already uses underscores as delimiter in many places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know that underscore is used a lot but with the use of the dash as a delimiter, it emphasizes the hierarchy in the form's ids wich is usefull when writing js for example

$('#post_author-name'); // with dash, i can say that this selector is related to post's author_name field
$('#post_author_name'); // with underscore, ... I don't know what is the relation

This is a silly example but I'm sure you understand what I'm trying to explain


return rtrim($clean, $delimiter);
}
}