#size for Select field documentation is wrong

See if this works.

Hi,

The patch seems to look good. Working fine.

There is a definite article missing in the first sentence. And, will any kind of number work? 4.256 is ok?

Fair point. Does the following sound more appropriate:
"- #size: A positive integer indicating height (per line) of the input element. This property can be used to expand the element to a scrolling list."

Maybe "line height" instead of "height (per line)" but yes, that's better.

Here's an updated patch.

Patch in #8 applies fine and the text content is clear and informative, marking as reviewed.

This was introduced in #2721725: Select form element has undocumented properties where the line was copy/pasted into many render elements.

Can we check the other ones added there and confirm that this fix is not needed for others? E.g. the one on the table element looks suspicious in that patch.

Unfortunately, #2721725: Select form element has undocumented properties did not get rescoped in quite the way that I asked at the time, so harder to check what else might be wrong, but #2486967: [meta] Move/Create Form Element Documentation and #2603810: [meta] See what else needs updating in Form/Render element docs are related metas.

Also, isn't the number of lines, not the line height?

The "line height" of this comment
is 18px
in the computed CSS.
But the number of lines
is 5.

Thanks @xjm for the review.

Here is a quick glance:

Tel, Email, Url, Pass
Documentation for #size correctly indicates number of characters. Would be good to add something like "This property can be used to control the element's width."

Select, Weight
#size indicates the height of the element per line.

Date, Table, Number, PasswordConfirm
No affect of #size (or is there something I missed :\ ).
^^
Need someone to verify these before rolling out another patch.

Please review the changes .

+++ b/core/lib/Drupal/Core/Render/Element/Select.php
@@ -38,7 +38,8 @@
+ * - #size: The size which indicates the width of element.

It should be "width of an element"

Sorry wrong patch.

I might have submitted patch too early, should have read the complete issue! My bad!

#size property for select is allowing us to configure the number of visible items in the drop-down list via the size attribute of the
tag. The last few patches don't mention this.

Should we say,
#size: Defines the number of visible options in a drop-down list.

I think still it's better to take this line from D7 ? - Size of multiselect box (in lines)

https://api.drupal.org/api/drupal/developer!topics!forms_api_reference.h...

I think the scope of the issue needs update in reference to xjm's comment https://www.drupal.org/node/2851204#comment-11930311

The discussion is going back from where it evolved.

@fotzulab: As per xjm's comment in #11 looks like this indeed got introduced via #2721725: Select form element has undocumented properties. Apart from #size for table and select elements, nothing looks fishy.

We might have to rethink #size for both table and select. For table it is mentioned, #size: The size of the input element in characters. which is not correct.

@swarad Here are some observations I had. You may want to cross check https://www.drupal.org/node/2851204#comment-11931596

I confirm, the #size has no affect on passowrd_confirm, number, table and date. #size does affect the select element, but the comment is wrong.

So now what exactly could be done with this issue?

@swarad07 I have tested your patch. Its working fine.

Tagging for upcoming contribution days.

Re-created the patch with just the changes on the documentation for the size attribute and for the 8.8.x branch.

Now that 8.8.x beta is out should we create this for 8.9.x?

Would be good to get this patch in because this documentation is so wrong for the size attribute

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21...

One line fix of an obvious error, looks good to me.

Fixing component

As per #11 and the research done in #14 we need to review all the elements where #2721725: Select form element has undocumented properties adding this documentation.

• So as per #14 we need to remove it from \Drupal\Core\Render\Element\Table - it is not supported see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/table.
• #14 also list Date, Number and PasswordConfirm. PasswordConfirm definitely supports and uses #size. Input elements support #size depending on type - see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#htmlattr....
• Date is complex because of the fallback to a plain textfield which does support #size - so I think the docs here need to reflect that it only affects the fallback.
• I think it should be removed from \Drupal\Core\Render\Element\Number too because as per https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#htmlattr... it is not supported.

So, now the scope of this issue is all Render elements mentioned in the #37?

Please review the patch.

- * - #size: The size of the input element in characters.
+ * - #size: indicates the height of the element per line.

Replace "indicates" with "Indicates".

Please review the patch updated.

I added the Needs issue summary update tag. I think adding the proposed resolution from @alexpott's review would help someone else review the patch in #41.

Removing last years contrib days - no use now!

Removing Novice - any issue with this much to and fro is not for a novice!

Very tempted to remove Need issue summary update as well: @alexpott I disagree with the suggestion that this issue should be expanded to check all elements in the way you have suggested. Please let us get this small win in, and if it is still appropriate/necessary create another issue to address that further work. At this rate, a one line docs change will end up taking >4 years and outlive D8.

I vote we accept the patch in #20 and move on, hence RTBC.

@rivimey but #20 is not even correctly formatted and is incorrect. It doesn't control the width of the select element. It controls the number of rows displayed. #34 is correct but a release manager in #11 explicit asked for everything to be checked. At the very least in order to commit #34 we should have follow-up issues created.

Reviewing #41...

+++ b/core/lib/Drupal/Core/Render/Element/Select.php
@@ -59,7 +59,7 @@
- * - #size: The size of the input element in characters.
+ * - #size: Indicates the height of the element per line.

This is incorrect - here #size decides the number of rows to display. I don't understand why the patch in #39 changed the wording from #34 which is correct. If we combine #34 with #39 ignoring the changes in core/lib/Drupal/Core/Render/Element/Select.php then as far as I see we have a patch that should be rtbc and committed.

Here I have merged patch #34 and #41. Please review it.

In Drupal Console, it uses "Size of multiselect box (in lines)" to describe it. See https://github.com/hechoendrupal/drupal-console-tl/blob/master/translations/generate.form.config.yml#L26

IMO, It's more clear than "The number of rows in the list that should be visible at one time."

Size of multiselect box (in lines) -- it indicates that the size is only applicable to select element which allows multi-select only.

Clear this comment and add a new one below

Size of multiselect box (in lines) -- it indicates that the size is only applicable to select element which allows multi-select only.

Sorry. my comment in #46 is incorrect. I've created a form with the code snippet below proved that I was wrong.

  /**
   * {@inheritdoc}
   */
  public function buildForm(array $form, FormStateInterface $form_state) {
    $form['number1'] = [
      '#type' => 'number',
      '#title' => $this->t('Number #size 5'),
      '#size' => 5,
      '#weight' => '-3',
    ];
    $form['number2'] = [
      '#type' => 'number',
      '#title' => $this->t('Number #size 15'),
      '#size' => 15,
      '#weight' => '-2',
    ];
    $form['select1'] = [
      '#type' => 'select',
      '#title' => $this->t('Select 1: #size 5, #multiple: FALSE'),
      '#options' => ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'],
      '#size' => 5,
      '#weight' => '-1',
    ];
    $form['select2'] = [
      '#type' => 'select',
      '#title' => $this->t('Select 2: #size 5, #multiple: TRUE'),
      '#options' => ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'],
      '#size' => 5,
      '#multiple' => TRUE,
      '#weight' => '0',
    ];
    $form['submit'] = [
      '#type' => 'submit',
      '#value' => $this->t('Submit'),
    ];

    return $form;
  }

No matter #multiple is TRUE or FALSE, the #size works for select elements. And #size in number elements as you can see, it makes no difference. For table element, I think it's the same with the number element. So patch #45 LGFM

Committed and pushed 9566f2017e to 9.0.x and 9efcbebcb8 to 8.9.x and d90b3261e7 to 8.8.x. Thanks!

Backported to 8.8.x as this is a docs fix.