Fix Helper Text space under Inputs to avoid layout flashes

[JulienMattiussi]

This PR makes the helper text space always reserved in forms inputs (even if there isn't any error or help text). This prevent forms to change height brutally upon validation, and the submit buttons to move under the mouse pointer.

Forms will be a bit taller than before with this change

[wmwart]

Uh. Whose interests does this change satisfy? And if I don’t have validation in my forms, if I’m not mistaken? Forms can be so large in height, and with this forced change will become even unreasonably higher. Strange decision. Is there a way to change helperText by default?

[DDushkin]

And also here is what happend with filters @fzaninotto

[fzaninotto]

I agree that this makes forms longer in general, and that it lowers data density, which is a pity.

But unfortunately that's how Material Design works. It's designed for Mobile devices, and error messages, which could appear on the side of the inputs on desktop, have to appear below.

As for disabling helperText if you don't use validation, this is a feature we can add, but it will be opt-in (something like <TextInput source="name" helperText={false} />).

The filters problem is a regression. I'll open an issue about it.

[fzaninotto]

Also, there are a number of past and present bugs that are due to the fact that the "submit" button may move down upon click (disabling the "click" and "blur" events) because of appearing error messages. The form layout shouldn't change upon submission.

[wmwart]

@fzaninotto, In materials material.io there is no mention of mandatory reservation of space for helptext for any of the platforms.

On the contrary, in the examples to text-fields it is shown that helptext appears when necessary, rather than being present all the time.

I think the best solution would be to add the default InputHelperText property to the form.

Also, there are a number of past and present bugs that are due to the fact that the "submit" button may move down upon click (disabling the "click" and "blur" events) because of appearing error messages.

The decision to place the submit form button just below the form was yours. And reserving space for helperText looks like a crutch.

[fzaninotto]

We decided to put the submit button at the bottom of the form because that's the most common in desktop UIs, and react-admin is mostly a desktop framework.

As for the material-ui guidelines, I read them differently than you. The spec says:

When text input isn’t accepted, an error message can display instructions on how to fix it. Error messages are displayed below the input line, replacing helper text until fixed.

And

Don't place error text under helper text, as their appearance will shift content.

The guideline on not shifting content is the one we're following here.

Also, the examples don't show a complete form with a submit button. The problem arises when the submit button is at the bottom of the form.

Again, I agree that the new solution (reserving space all the time) is not perfect, but neither was the previous one (never reserving space). We had to make a decision, and we think the new solution lets you have a denser layout when you need it (with helperText={false}) without harming User Eperience in the broader case.

I'm not sure I understand your suggestion:

I think the best solution would be to add the default InputHelperText property to the form.

Can you please elaborate?

[djhi]

I think the best solution would be to add the default InputHelperText property to the form.

I think it's a prop inputHelperText on the form accepting a component to replace the default one.

[fzaninotto]

ugh. Another property to inject to all form children... Not a big fan.

[wmwart]

Yes, that’s what I meant. Translation difficulties)
something like:

SimpleForm.js

import InputHelperText from '../input/InputHelperText';
...
SimpleFormView.defaultProps = {
    submitOnEnter: true,
    toolbar: <Toolbar />,
    helperTextComponent: <InputHelperText />
};

nother property to inject to all form children...

But you have already done not just injection of the property to all children, you have changed all the input components...

[fzaninotto]

I'm not complaining about the work it'd represent for us, but about the added complexity and coupling between forms and inputs. Not to mention another cloneElement, which we're currently trying to reduce for performance reasons. Also, the philosophy of react-admin isn't to offer components that you can customize in every possible way. It's to offer components that you can easily replace with your own.

If you're not satisfied with the way react-admin Input components render, you can still use your own variation. It's as simple as

// in src/myapp/form/TextInput.tsx
import { TextInput } from 'react-admin';

export default props => <TextInput helperText={false} {...props} />

[nik-lampe]

I totally understand that you had to make a decision, and it is a trade-off in either way.

But your proposed solution does not for me like it should. The "empty" helper text component is still displayed even with helperText={false}

I looked into the code, and the problem seems to be, that, for example the TextInput Component injects the InputHelperText component into the helperText Prop of the TextField.

  helperText={
                <InputHelperText
                    touched={touched}
                    error={error}
                    helperText={helperText}
                />
            }

This Component receives the helperText as prop, and inside it checks if it is false (and touched is not true and error is not false) and then returns null.

 return touched && error ? (
        <ValidationError error={error} />
    ) : typeof helperText === 'string' ? (
        <>{translate(helperText, { _: helperText })}</>
    ) : helperText !== false ? (
        // material-ui's HelperText cannot reserve space unless we pass a single
        // space as child, which isn't possible when the child is a component.
        // Therefore, we must reserve the space ourselves by passing the same
        // markup as material-ui.
        // @see https://github.com/mui-org/material-ui/blob/62e439b7022d519ab638d65201e204b59b77f8da/packages/material-ui/src/FormHelperText/FormHelperText.js#L85-L90
        // eslint-disable-next-line react/no-danger
        <span dangerouslySetInnerHTML={{ __html: '&#8203;' }} />
    ) : null;

but I assume that this still creates a component, and thus the empty Helpertext is still displayed even with helperText={false}.