XLSForm / pyxform

A Python package to create XForms for ODK Collect.
BSD 2-Clause "Simplified" License
80 stars 136 forks source link

683: select or_other label is "-" in multilanguage form not "Other" #687

Closed lindsay-stevens closed 8 months ago

lindsay-stevens commented 8 months ago

Prior to using secondary instances for all selects, the default label for the or_other shortcut was "Other" in all languages. Despite the various warnings, users wanted the old behaviour to be retained. The exception is that if a user has defined an "other" choice for the or_other choices, then any blank translations labels ("-") will not be replaced with "Other".

Closes #683

Why is this the best possible solution? Were any other approaches considered?

I tried to find a way to offload the logic to survey.py because that is where translations and missing defaults are done, but it became quite messy. Basically the OR_OTHER choice had an extra attribute "_or_other": True and this was used to trigger special processing when setting up translations and overriding the default missing translation -.

This PR is similar to #684. It seems like builder.py is the best place to add this code because it's where the or_other input item is injected, and where the regular or_other choice is added. Differences from #648:

What are the regression risks?

The only one that comes to mind is that as mentioned above, if a user has defined an "other" choice for the or_other choices, then any blank translations labels ("-") will not be replaced with "Other".

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No

Before submitting this PR, please make sure you have:

lognaturel commented 8 months ago

users have no way to apply filtering for the other choice since it doesn't exist in the form.

I imagine "Other" would generally be desirable for any filtered list. But we also shouldn't expand the capabilities of functionality we want to discourage so I agree with taking it out.

The or_other related tests are put into a new TestCase class, to help visually separate them from others.

Yes, great.