Placeholders should not be created automatically

[Floriferous]

The logic for displaying placeholders seems to be more complicated than necessary.

Reading through this bit of code and PR #152, makes your head spin for a fairly simple feature.

I think this package should not be generating placeholders automatically for people, as it is too opinionated. And there are a few other important reasons:

• Using the same placeholder as the label is a bad practice: Your label should describe what is in the field, and the placeholder should give an example of what to put in it:
  • label: City, placeholder: New York
  • label: Amount, placeholder: $0.00
  • label: Address placeholder: Address is not helpful for anyone, and repetitive
• Any app dealing with i18n will not be able to use the feature, we should stop assuming everything is written in a single language, or written in English only: When you call your field firstName, only a small portion of the world actually wants to see First Name as the label, and even worse: First Name as placeholder (given UX guideline above). It's cool to be able to display something fast for single-language MVP apps, but that's about it.
• Auto placeholders fail if you do anything a bit more complicated with your labels. For example, we're passing the label directly to the AutoField component as a prop, not via the schema. So all our labels are well translated, but the placeholder still shows First Name.
  • If your labels are React components, no solution will work anyways because the placeholder prop expects a string, or else it displays [Object Object]

I think placeholders should be displayed if a placeholder is provided on the component/schema-field and that's it. Maybe add another prop called autoPlaceholder that generates them for you (which I recommend nobody actually use anyways), but the current status isn't great.

We don't have placeholders for all our fields right now, so I'm left looping over my schema and give my fields empty placeholders to avoid the automatic unwanted ones?

[radekmie]

Hi @Floriferous. Let's get through it step by step.

The logic for displaying placeholders seems to be more complicated than necessary.

There's literally zero logic for displaying them, but there is an already mentioned flowingProp function for determining them, based on props, schema, and current context. I also disagree, that it is not a complicated task: you can enable, disable and override on each of these three levels (remember about using null, which has a special meaning).

If you have an idea on how to simplify it - I'm open to suggestions and PRs.

I think this package should not be generating placeholders automatically for people, as it is too opinionated.

That's why it's disabled by default. If you are using a schema, which, somehow automagically, determines a correct placeholder, then using current logic you have an out-of-box solution, using placeholder={true} on your form. It's not so uncommon to use a custom bridge (I've actually written more than ten) and wiring up an external translation system is not a big deal.

Using the same placeholder as the label is a bad practice: [...]
Any app dealing with i18n will not be able to use the feature, we should stop assuming everything is written in a single language, or written in English only: [...]

I agree. But again, provided that your schema contains a good placeholder, the current solution is a very good one. A problem for me, as a creator of this package, was to support most of the cases and not break the others. If such a thing was a goal here, I think that the current solution is good, as it's providing some placeholders for a quick look (and potentially a finished feature) and not getting in your way, if you plan to write your own.

Auto placeholders fail if you do anything a bit more complicated with your labels. For example, we're passing the label directly to the AutoField component as a prop, not via the schema. So all our labels are well translated, but the placeholder still shows First Name.

OK, that's new. It was a decision a long time ago, and labels are currently only strings (placeholders too). It's really simple to fix that with an additional condition (type check) in flowingProp, but no one raised this issue. I'm not sure (yet) if it's a good solution, but it's possible.

If your labels are React components, no solution will work anyways because the placeholder prop expects a string, or else it displays [Object Object]

This is actually a thing of your theme - placeholders can be used anywhere, for example in a tooltip.

I think placeholders should be displayed if a placeholder is provided on the component/schema-field and that's it. Maybe add another prop called autoPlaceholder that generates them for you (which I recommend nobody actually use anyways), but the current status isn't great.

Maybe it's an idea to actually treat schema-level placeholder as a trigger to actually apply it. It would be a breaking change though, as if your schema actually returns a valid placeholder, it would be turned on by default now.

We don't have placeholders for all our fields right now, so I'm left looping over my schema and give my fields empty placeholders to avoid the automatic unwanted ones?

If you can't do it globally with your schema (a default placeholder of false in each field) then yes, it is a manual process.

[radekmie]

No answer so far, I'm closing. Feel free to comment anyway.