Closed paregorios closed 2 years ago
Is this a blocker for #371?
This is not a blocker for #371.
@paregorios Looks like an easy fix to the selection widget used on Connection, Location, and Name. It also looks like a few other fields on Name are missing the functionality: "Level of transcription completeness", "Accuracy of transcription", "Name type", and "Language". Should I update those as well?
@alecpm yes, please update all fields from which the functionality is missing.
@paregorios this fix is applied on staging. There is an interesting quirk though:
When the AT selection widget has 3 or fewer values it uses a radio widget by default, and with more than 3 it uses a select widget. Our custom version which filters hidden vocabulary items includes the hidden values in the count when determining the style of widget, so adding a new hidden item can change the widget from a radio to a select without any new items appearing. I don't think that's a big problem, but for some of these widgets with relatively few values we may wish to configure them to always use a radio widget rather than allowing the default auto-selection behavior.
Design guidance from M$ opines that for option count <= 7, one should use radio buttons, otherwise a drop-down or single-selection list.
@alecpm Is it practical and low-cost for us to renovate the AT selection widget behavior across Place, Name, Location, and Connection content types at this time as part of the work on this ticket, or should we break that out as a separate issue?
If we want to simply say that specific fields on specific types will always be a radio (because we expect to have fewer than 7 values) or a select (because we expect to have more than 7 values), then this is a pretty trivial change. If we want to fix our custom widgets to automatically determine the input type based on the item count while factoring out the hidden items, that's a more extensive change. Either one could be considered a part of this ticket, though I'm not sure I have enough time this week to do the latter one.
ok @alecpm for fields governered by vocabularies that currently have <= 5 values, let's go with the trivial solution to make the field always be a radio. Do you need me to do the summing and reporting?
I can manage that.
I've updated the widget format to radio
for "Association Certainty", "Attestation Confidence", "Name Accuracy", and "Name Completeness". The only other short vocab is "Location Type" but that's only used in a multi-select.
Sounds good. Let me know when this is ready for staging review.
@paregorios Sorry should have been clearer, but it's already live on staging.
@alecpm is it in-scope for this ticket to make this functionality work on:
@paregorios I think so. The FilteredSelectionWidget
is already being used on the relationshipType
(Connection type
) field on Connection
, so there's another bug lurking here. I'll take a look.
@paregorios Actually there's not really a bug here per se. The field default value is hardcoded to connection
which is why it always appears in the vocabulary on the add form (it would go away on the edit from if you had selected another value). Is there some other value you'd like to use as a default? Should there not be a default?
While trying to track this down I ended up implementing your suggestion of auto-switching the widget type based on visible vocabulary length >= 7, which turned out to be more trivial to do than I had expected.
@alecpm if we can safely go to a state where there is no default for this widget, I would prefer that.
@paregorios It's about the specific field in this case rather than the widget more generally. In this case not having a default value would mean that the select would populate by default with the first value alphabetically (currently at
). If that's not desirable, I could implicitly add an empty/None option to only this vocabulary, which would trigger a validation error when selected.
@alecpm thinking about the fact that this vocabulary may grow, I'd rather not have an arbitrary value appearing first in the list down the road. Honestly, "at" is a very good default value, so let's replace the current hard-coded default "connection" with a hard-coded default "at".
This fix is live on staging now.
This is working perfectly on staging. Please merge to master and deploy to production.
this is working perfectly on production.
Bug discovered on staging while reviewing #371:
Steps to replicate:
Expected behavior
For reference, the following vocabularies have been tested today and they work in the manner expected: