Steps to create:
1. Download the module
2. Create a web form.
3. Under wizard settings, by default it looks empty.

Proposed solution

--None-- or --Select-- can be added in the drop-down.

Note: Most of the dropdowns have empty space and it does not look good. Sorry for being pedantic about the UI-text :)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Maheshwaran.j created an issue. See original summary.

jrockowitz’s picture

Issue summary: View changes

I am not sure I want to change this.

The tricky part is that most users should not be setting these select menu values and '- None -' or '- Select -' might entice them to set a value.

We could use - Default (value) -.

Maheshwaran.j’s picture

So instead of

'#empty_option' => t('- None -'),
'#empty_value' => '_none',

We can go for

'#empty_option' => t('- Default -'),
'#empty_value' => '_none',

Or

If you are suggesting to leave a default value, it's fine, but leaving an empty box for empty value does not look good when a normal user/client is trying to create a web form. Besides the field is not required. So I think we can give an empty option for site builders or clients.

Thanks :)

jrockowitz’s picture

I think we would have to go with just

'#empty_option' => t('- Default -'),

To make this change we would need to look at all the '#type' => 'select' element

'#empty_options' => '',

... and #options for...

'' => ''

Maheshwaran.j’s picture

Assigned: Unassigned » Maheshwaran.j

I'll look into this

Maheshwaran.j’s picture

Assigned: Maheshwaran.j » Unassigned
Status: Active » Needs review
FileSize
6.19 KB

I have created a patch for this. Please review this.

jrockowitz’s picture

I did a full review of all the select menus as established the below convention.

Required: - Select -
Optional: - None -
Default: - Default -

I also converted < Default > to - Default -

Status: Needs review » Needs work

The last submitted patch, 7: 2934087-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jrockowitz’s picture

Status: Needs work » Needs review
FileSize
22.68 KB

Status: Needs review » Needs work

The last submitted patch, 9: 2934087-9.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jrockowitz’s picture

Status: Needs work » Needs review
FileSize
21.89 KB

I am stumped because the failing test is passing locally.

The attached patch is just a guess.

Status: Needs review » Needs work

The last submitted patch, 11: 2934087-11.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jrockowitz’s picture

Status: Needs work » Needs review
FileSize
18.42 KB

I reverting all changes to \Drupal\webform\Form\AdminConfig\WebformAdminConfigElementsForm in the hope that the tests pass so that I can commit this patch. Then we can update WebformAdminConfigElementsForm.

Status: Needs review » Needs work

The last submitted patch, 13: 2934087-13.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jrockowitz’s picture

Status: Needs work » Needs review
FileSize
22.68 KB

The last submitted patch, 9: 2934087-9.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • jrockowitz committed e382de0 on 8.x-5.x
    Issue #2934087 by jrockowitz, Maheshwaran.j: While Creating a form ,...
jrockowitz’s picture

Status: Needs review » Fixed

I committed the patch. Please download the latest dev release to review.

This was the regression #2934542: Fix broken Webform.Drupal\webform\Tests\WebformLibrariesTest that was causing the failing tests.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.