Add defaultProps to `SingleSelect` and `MultiSelect` so there are default text available for components

[HendrikThePendric]

From what I could tell, both the MultiSelect and the SingleSelect need defaultProps set for the same props, which are listed below.

clearText (Clear)
filterPlaceholder (Filter)
loadingText (Loading)
noMatchText (No results found)
placeholder (Please select an option / Please select one or more options)

@Ismay could you check:

• If the above list of props that need default values is complete?
• If the default values in parenthesis are OK to you? Pls correct where needed.

Probably good to get feedback from @varl on this as well. I remember us discussing where to have translations for the Select, but did we settle on how to approach it in the end? Would ui-widgets and ui-forms be appropriate for our translations for example? Also I'm not sure if there's anything in the works for the app shell and how it would interact with translations.

As for the Select properties: yes, those are the ones. Though I can also imagine that people wouldn't want them set. Maybe we should just pass them along somehow?

[HendrikThePendric]

but did we settle on how to approach it in the end?

Yes ui-forms and ui-widgets are the appropriate places to add translatable texts.

Though I can also imagine that people wouldn't want them set

My knowledge of the *Select may be too limited... It wasn't my intention to set default values for textual props that make something appear. I.e. clearText won't do anything unless clearable is also true, so in this case providing a default value is fine, right? If a different text is needed, the app-developer can just specify a clearText prop and that will be overridden.....

Looking at the *Select code, I can see that the placeholder will cause this text to show up unconditionally, so I guess we'd better not provide a default value for that.

@Ismay could you make an educated guess as to which textual props should/ shouldn't get default values?

If we only want to set default text for text that requires a feature to be toggled to be displayed these are the texts you want:

• clearText (needs clearable)
• filterPlaceholder (needs filterable)
• loadingText (needs loading)
• noMatchText (needs filterable)

I'm thinking though, maybe it's good to house all the translations in ui-widgets for example? Otherwise it might be awkward that we have form components translated here and in ui-widgets. Maybe someone wants to use translated form components, but not FF for example. We could then import the translated components from ui-widgets to ui-forms.

[HendrikThePendric]

@Ismay thanks for that list of props that should get translated strings.

Regarding your suggestion for creating a ui-widgets component that holds translations: I'm not sure about this. On the one hand I think your line of reasoning is perfectly valid and thus would justify creating a component in ui-widgets. But on the other hand, I fear that then having three virtually identical components in three different repos could possibly cause more awkwardness than the amount of awkwardness we are trying to avoid.

I do seem to remember that at some point we agreed that the places to add translatable strings to components would be ui-widgets and ui-forms. If we head into the direction you're suggesting, then I think we might as well remove i18n from all the components in ui-forms (we'd still need it for the validators).

@varl @Mohammer5 Please share your views...

[varl]

If the direction is that ui-forms is a "headless" library and just wraps components from ui-core and ui-widgets, I can see the UI translations in widgets.

[HendrikThePendric]

If the direction is that ui-forms is a "headless" library

Yeah.... It could make sense to head into that direction (pun unintentional) but to my understanding that wasn't the original approach we agreed on....

I do seem to remember that at some point we agreed that the places to add translatable strings to components would be ui-widgets and ui-forms

Having said that, I've reviewed the code-base and the only other component that currently holds translations is the FileInput. And there is already an issue that requests moving the FileInput to ui-core https://github.com/dhis2/ui-core/issues/651. In my mind, this issue could then be replaced by an issue in ui-widgets titled Move FileInput from ui-forms to ui-widgets. So going the headless way would not be a big job.

BUT, even though it could make sense to move all the translations out of ui-forms on a conceptual level, I do still think it makes things really confusing for the app-developer. If we use the MultiSelect as an example, the app-dev will need to be aware of the following:

• When building something custom, or if fully custom texts are needed, and not using React Final Form => ui-core MultiSelectField
• When using the component directly, without React Final Form, and the dev wants to use the default texts => ui-widgets MultiSelect
• When using React Final Form => ui-forms

And just to compare that to the choice the dev would have to make if we would not introduce these components to ui-widgets purely for translations (i.e. keep having translations in ui-forms components):

• Without React Final Form => ui-core
• With React Final Form => ui-forms

Obviously this is a much clearer distinction, and would require the app-dev to maintain a much simpler mental model about how our repos relate to one another...

So in mind we are weighing up pros and cons again. Lets try to "quantify" them for the MultiSelect:

• Advantage of keeping the translations in ui-widgets: the app-dev doesn't have to provide texts for the 4 props mentioned in Add defaultProps to SingleSelect and MultiSelect so there are default text available for components #211 (comment) when the MultiSelect is being used without React Final Form.
• Advantage of keeping the translations in ui-forms: less confusion for the app-dev

What do you think @varl @Mohammer5 @Ismay... Go "headless" or not?

[HendrikThePendric]

After re-reading what I wrote above, I feel like I have neglected to mention the fact that in all likelihood the app-dev will be ui-widgets components already when building an app. So he/she will already need to have some working mental model in place of what distinguishes a ui-core component from a ui-widgets component.

So my comment above might have a bit of an unfair bias towards the "keeping i18n in ui-forms" solution.

Also, good documentation can go a long way here. We could even list the differences explicitly in all repos...

Yeah.... It could make sense to head into that direction (pun unintentional) but to my understanding that wasn't the original approach we agreed on....

The way I see our intention with this library is that we want it to be a transparent (or as transparent as possible) layer between our components and react final form. So to me, the scope of this library is very minor: a small shim that makes our components conform to the api that a react-final-form field expects (combined with some transform and validation helpers).

That keeps it maintainable for us, and understandable for devs using this lib as they're just using RFF in the end. I think adding translations to this lib stretches the scope of this lib somewhat, that's why I'd prefer not doing that.

BUT, even though it could make sense to move all the translations out of ui-forms on a conceptual level, I do still think it makes things really confusing for the app-developer.

To me the distinction makes sense if you look at it like this:

• ui-forms: convenience wrapper that wires up our components to react-final-form
• ui-core: basic building blocks (no translations)
• ui-widgets: more elaborate components (including translated ones)

Since ui-forms is meant for convenience I'd default to the translated components from ui-widgets there. The dev will then still be able to override the translations where necessary by setting the props they want to override on the Field. That keeps all the translations in one place, and scoping-wise seems clear to me as well. What do you think @HendrikThePendric?

[HendrikThePendric]

What do you think @HendrikThePendric?

I am warming to the idea, but I think this is actually a fairly substantial decision to make, so I'd like to get some input from at least one other team member before we proceed with this.

[varl]

Going headless to me would mean that ui-forms would not have a dependency to ui-core/-widgets at all. Do we share that definition?

Thinking on this some more, I think that ui-forms being headless is the "right" way to go here:

1. It makes the separation of concerns between ui-core / -widgets / -forms very clear.

2. ui-forms could be used in situations where a hybrid approach is needed (e.g. you have some custom components, some core components, and some MUI components), which is the case for almost every application in existence.

3. ui-forms hasn't been released yet, so we can still do this without breaking the world.

Question I don't know the answer to: how would a headless ui-forms library look in practice?

@varl I'm not sure what you mean by headless, do you mean that this lib should be able to wire up not just our components to RFF, but also MUI and custom components?

[varl]

@Ismay "headless" to me means that it doesn't render anything by itself, so yes.

This is mostly to sort out the terminology we use so we all share the same understanding:

E.g.

<Input
  component={InputField}
  onChange={}
  onFocus={}
  ...props
/>

We'd tailor it to fit our API, but if you want to use a MUI field instead, you'd just have to pass in a callback to the handlers with the signature that MUI components expect (event instead of obj, event).

[HendrikThePendric]

I like the notion of being a headless lib, but as I see it some things stand in our way:

• It's hard for us to be a headless form lib, because essentially that's what RFF is (i.e. RFF Field even has a component prop like the example @varl provided).
• We could explore an approach in which we could be a generic adapter (via a component or function). We'd have to be a layer between a single type of parent, the Field, and a multitude of input components, each with their own API. I don't see how we can do this in a generic way.
• So we could also explore a more granular way, having smaller dedicated helpers for individual props. And that's essentially what we are doing already in our components (although we are not exposing these helpers yet). But I think this approach won't result in a sufficient level of convenience for our app developers: too much boilerplate would be required to just wire up an input for an app.

So, as a general approach, I like the headless suggestion, I just don't really see how we'd do it. Perhaps someone else does...

If you look at the code in the ./src/components folder, you will see that all components contain very little logic (except FileInput, but ignore that one, it will be moved to widgets/core): we're just mapping some props and computing a few of them via helpers. So making a MUI-based text input component would be equally easy for an app-dev. Ultimately, we want to offer a comprehensive set of components, all using the DHIS2 design system, so the app-dev won't have to take that step anymore. But until then, it's pretty easy to build one in the app, especially if we were to expose our helper functions (e.g. hasError, isValid etc.).

I guess what I am trying to say is, atm we are not headless, but our components are just a very thin layer between the RFF Field and the ui-core components. I think that's not bad either.

[Mohammer5]

I have to agree with @HendrikThePendric, I don't really see a way to turn ui-forms into a headless library, as that's essentially what RFF already does.

I like the idea of having all translations (and therefore a translated component for each input component in ui-core) in ui-widgets.

Right now ui-core is a peer dependency of ui-widgets, so a non-breaking change in ui-core could be used in apps right away once that change has been released, same goes for ui-forms, which currently has ui-core as peer dependency, but once changed to ui-widgets, any non-breaking change in either ui-core or ui-widgets could be used right away once the change has been released.

Maybe we could give the ui-forms components a prop equal to the name of the component but pascal-case, which holds the component to be used and defaults to the ui-widgets component, here's an example:

import { CheckboxField } from '@dhis2/ui-widgets'

const Checkbox = ({
   // ... all existing props
   checkbox: Component,
}) => (
    <Component
        // ... all existing props
    />
)

Checkbox.defaultProps = {
    checkbox: CheckboxField,
}

This way an app could still use custom components which implement the same api as the ui-core components without having to do all the "magic" that's happening in ui-forms themselves (which eventually means that if that app moves away from RFF, the custom componenents' api still matches the ui-core components' apis.

[varl]

Sure. I'm bringing up the case now, since an immediate situation we saw with ui-core is that devs want to combine it with other components, so we need a way to build forms using a combination of "any components". If we do not easily have that ability then ui-forms practically cannot be used in "mixed apps" without dropping down to installing RFF as a direct dependency (no-go).

If RFF is already headless and can be used with any old component (makes sense), then maybe ui-forms needs to rethink its scope to stay that way, and expose things that RFF does not give us (validators, etc.)?