[Forms] Selects coming up blank

[VWoeltjen]

@charlesh88 reports that on master at 8c1b70f, selects (specifically, mct-control where key is select) are not being displayed appropriately.

Appears that this relates to the line:

<option value="" ng-if="!ngModel[field]">- Select One -</option>

This condition is intended to add a "select one" pseudo-option for the case where the property described by the control has not had a value defined.

[VWoeltjen]

Guessing this is related to #81 which updates the version of Angular. It's suspicious to me that there's both an ng-options and an actual <option> tag present, seems that this may conflict.

[larkin]

Hrm, I thought that ng-options and an options tag was explicitly supported to allow for customizing the "select one" message. I'm guessing the ng-if causes problems here.

I believe you can set the <option> element to disabled so that it can't be selected if another element has been selected, but it would still be the default value if it's the first value in the options list.

[VWoeltjen]

Yeah, ng-if does seem to be part of why this issue occurs.

Desired behavior here is that the "- Select One -" should disappear once the user actually selects something. Changing the ng-if to an ng-show gets the desired behavior, will PR with that

[charlesh88]

Re. 'Desired behavior here is that the "- Select One -" should disappear once the user actually selects something': this is true only if the field is required. To be fair, "- Select one -" should only be included if the field is required; otherwise, the default should be displayed and would be considered a valid option. I don't think we have this level of functionality built in yet, however.

[VWoeltjen]

@charlesh88 I don't disagree, but I'd like to use that as a design guideline instead of a behavior of the control, where the design guideline is "if choosing an option from a set is not required, provide a default value" and the control's behavior is as described ("- Select One -" is shown when no default value has been assigned). This is assuming that the design rationale for not having "- Select One -" is that some default will be enforced instead, and the user should be able to see the default that will be used.

Mostly, this is to keep the control simple and consistent, from a developer perspective. I also think it's true that the control won't be very effective at picking sensible defaults (it would take the first item in the list, but the list order has nothing to do with general relevance; e.g. "pick a country")

What we could do here is issue a warning to the developer console if there's no default value provided - build a little bit of design guidance into the control while keeping it functionally simple. Console noise gets spotted eventually during development so I think this would be more effective in practice at getting sensible defaults to appear in those selects.

[larkin]

This is just a select field, right? Seems we're over engineering something that every user agent already has put significant effort into making easy to use.

We should be careful with dynamically updating the list of options-- in fact I'd advocate against it. The odds that we break functionality on chrome or mobile or some other user agent are high.

There are many webforms with horrible select field functionality because they tried to add special handling and broke basic interactions. Ever been on a checkout screen where you can't tab through a form? Or where typing to select values in a select field doesn't work? It's the worst.

The robust and simple solution is to add a disabled attribute to the "select one" option so that while it will be the default value (if we don't set a value ourselves), the user can't actually select it. If a field is required, you add some decoration to the field (probably near the label), and display a warning if they try to submit without a value. No dynamic updates necessary.

While we're at it, we should fix the bug in our forms where pressing "enter" does not submit the form.

[charlesh88]

Think about it like this: instead of a select, picture instead a group of radio buttons. I think 99% of the time we can design the form such that this element has a default value that makes sense, usually something like "All" or "No preference". So that default radio button would be pre-selected when the user first encounters the form. Even though that radio group might be "required", it can't invalidate because there's no way to unselect a radio button. And, there is no radio button labeled "- Select one -".

However, there can often be a case where the default isn't good enough - we need a human to consciously make a choice, and know that they did. In that case, no radio buttons would be pre-selected, and the form would not validate unless the user clicks a radio button in the group.

I don't really think we currently have a use case for the latter. If we could allow a configurable default in the JSON config for that element in the select's option group to support the first case, I think that works. For example, in the current Create Panel dialog, we allow the user to select the default view (plot, fixed pos, or scrolling) via a select that initially displays "- Select one -". What we should really be doing here is to remove "- Select one -" and just default to "Plot" - which is what happens "for real" (quotes because the default view isn't yet actually implemented).

[larkin]

We definitely want configurable defaults.

Here's our current select syntax:

{
    "name": "Default View",
    "control": "select",
    "options": [
        { "name": "Plot", "value": "plot" },
        { "name": "Scrolling", "value": "scrolling" }
    ],
    "key": "defaultView"
}

I propose this:

{
    "name": "Default View",
    "control": "select",
    "options": [
        { "name": "Plot", "value": "plot" },
        { "name": "Scrolling", "value": "scrolling" }
    ],
    "key": "defaultView"
    "placeholder": "wont be displayed because of default value"
    "default": "plot",
    "required": true
}

The three new values would work like so:

• "placeholder": placeholder text. most controls support the concept.
• "default": default value for field. if blank, placeholder will be shown.
• "required": if true, add styles to the field label to indicate it is required (the ubiquitous red *).

So any given select field would have:

• placeholder -- a disabled select option, can't be selected, but hints at what can be typed. Again, only included if configuration specified a placeholder.
• n number of selectable options.

If a field is not required and the user should be able to "select none", then an option should be added for "none", and optionally, set as default. Selecting a blank value in a select field is a really weird interaction-- people don't click on empty space.

Thoughts?

[VWoeltjen]

So, we have configurable defaults already - you can provide an initial state for forms. This is separate from the form definition, which I think is an appropriate separate concerns: You have one object defining the structure of a form, and another for the state itself. Sticking defaults in the form definition starts mixing content and structure (and creates an additional degree of redundancy.) I think our options and validator properties are also straddling the line between structure and content (by describing "possible content") but I'm happy to let that continue keeping me up at night.

As for the other options, the required property is supported already (maps to ng-required IIRC). I do think placeholder makes sense as an addition.

This discussion is also getting pretty fair afield from the original issue. Is there a separate issue that we should be filing?

[larkin]

Back to original issue:
I think we should remove the ng-show directive and disable the option, instead of dynamic hide/show behavior based on the user selecting a value. Every browser implements form controls differently, and I'd prefer to let them do their thing-- it's safer and simpler.

Back to far afield:
I think a ticket for discussing "placeholder" behavior would be a good place to move this discussion.

[charlesh88]

Migrating this discussion to a new issue: #125