marmelab / react-admin

A frontend Framework for single-page applications on top of REST/GraphQL APIs, using TypeScript, React and Material Design
http://marmelab.com/react-admin
MIT License
24.8k stars 5.22k forks source link

TabbedForm validation differences between development and production build #5428

Closed ronaldtb closed 3 years ago

ronaldtb commented 3 years ago

What you were expecting: I was expecting the FormTab instances in a TabbedForm to turn red in case any of the child inputs are invalid. This is working as expected during development. The className "RaTabbedForm-errorTabButton-41" is being added to the FormTab.

Opening the page containing the 'Create' and 'TabbedForm' component shows all tabs as invalid even before pressing the save button.

What happened instead: After running yarn build and deploying this production build the tabs are not being marked as invalid. The inspector is showing a className "jss40", but this class can't be found in the generated sources.

The tabs are not turning red on validation errors.

Other information: Dev environment (yarn start) ReactAdminDev

Prod environment (yarn build) ReactAdminProd

Environment

djhi commented 3 years ago

Our demo (https://marmelab.com/react-admin-demo/#/products/create) is a production build and the tabbed forms do show invalid tabs with correct styles so I believe there is an issue with your setup. We had similar issues in the past and they were all related to multiple conflicting versions of a dependency. Please double check yours, especially packages related to react-admin and material-ui.

ronaldtb commented 3 years ago

Thanks for the quick response and I will check the demo and our dependency versions.

fzaninotto commented 3 years ago

This issue has gone silent. Without any way to reproduce the issue, we have to close it. Feel free to comment with new info if you want it reopened.

donovantc commented 3 years ago

@ronaldtb did you discover which packages were conflicting? I'm having a similar issue. Except the css is there but the MUI css is overriding the jss (react-admin css classes).

I'm using:

"react": "^17.0.1",
"react-admin": "^3.12.1",
"react-dom": "^17.0.1",
"react-scripts": "^4.0.1"
donovantc commented 3 years ago

I've been struggling with this issue for a while now, so I decided to create a very basic setup using create-react-app and add react-admin as mentioned in the react-admin tutorial.

I added a create component which has 2 tabs, and set all fields to required. In development, if you start entering a field, but leave it empty and switch to the next tab, you already see the previous tab with red text.

image

However in the production build, this doesn't happen. The text remains its original colour.

Here is the codesandbox with this basic setup. Here you will see the correct behaviour in the development build.

I have also deployed this code directly from the codesandbox via Netlify: https://csb-m5c9z.netlify.app/#/users/create. Here you will see the incorrect behaviour in the production build.

Looking at the css in the production build, you can see that the color from react-admin styles jss91 is overwritten by the MuiTab-TextColorInherit class.

image

Looking at how they are included in the final build, the MuiTab styles are loaded after the RaTabbedForm styles, meaning they will get preference.

image

So I think this may be related to how the CSS is imported or loaded.

@fzaninotto @djhi unless I am doing something wrong, I believe that this is still an issue.

vishalb812 commented 3 years ago

As a temporary workaround, the errorTabButton color can be overriden with the !important property. Code snippet below:

import { makeStyles } from '@material-ui/core/styles';

const useStyles = makeStyles(
  theme => ({
      errorTabButton: { color: "#f44336 !important" },
  }),
  { name: 'RaTabbedForm' }
);

  const List = () => {
    const classes = useStyles();

    return (
      <TabbedForm
          tabs={<TabbedFormTabs scrollable={true} />}
          toolbar={toolbar}
          {...rest}
          style={{ width: "100%" }}
          decorators={decorators}
          classes={classes}
          warnWhenUnsavedChanges
        >
            ....
        </TabbedForm>
        )
   }
otroboe commented 3 years ago

Hey! I have an issue as well, a difference between development or build mode. I have an error in dev mode:

Material-UI: The key `errorTabButton` provided to the classes prop is not implemented in undefined.
You can only override one of the following: root,line,index,form,action,leftIcon. 
    at SimpleFormIterator (http://localhost:3000/static/js/vendors~main.chunk.js:128871:18)

But no error when I build the app.

fzaninotto commented 3 years ago

The demo seems to allow the reproduction of the error, so I'm reopening the issue.

https://marmelab.com/react-admin-demo/#/products/create

fzaninotto commented 3 years ago

Should be fixed by #5984

donovantc commented 3 years ago

@fzaninotto @djhi I tested this change with the new version 3.13.2 of react-admin but I still face the same behaviour in production builds as mention in my previous comment https://github.com/marmelab/react-admin/issues/5428#issuecomment-776021213

I updated the version in that same project (mentioned in the previous comment). It works fine in development but when I deploy a production build, the behaviour remains incorrect.

Based on this, I don't think #5984 fixes this issue.

djhi commented 3 years ago

The demo is using the production mode and I can't reproduce the issue on it. Please double check the version of all react-admin packages

donovantc commented 3 years ago

I also couldn't reproduce it in the live demo version, but this was true even for before the fix in #5984. So I suspect that the demo version might not be using the latest version of react-admin. Please correct me if I'm wrong?

I don't know internally if you update deps and rebuild the live demo version after each release? In the source code for the demo an older version (^3.9.0) of react-admin is specified https://github.com/marmelab/react-admin/blob/58e00dc022ec57599855b05367183c62bf798cbd/examples/demo/package.json#L23. And there is no package or yarn lock, so I can't say for sure which version of react-admin is being used in the live demo. Hence it's just a suspicion 😄

Dreamprogyang commented 3 years ago

My version is v3.13.3. But sadly I also face the same behaviour in production builds as mention in the previous comment #5428.Just as same as before.It works fine in development but when I deploy a production build, the behaviour remains incorrect. well,I also don't think #5984 fixes this issue.

fzaninotto commented 3 years ago

I just redeployed the demo (v3.13.4) and it still doesn't show the problem. Please provide a way to reproduce it.

donovantc commented 3 years ago

I've been struggling with this issue for a while now, so I decided to create a very basic setup using create-react-app and add react-admin as mentioned in the react-admin tutorial.

I added a create component which has 2 tabs, and set all fields to required. In development, if you start entering a field, but leave it empty and switch to the next tab, you already see the previous tab with red text.

image

However in the production build, this doesn't happen. The text remains its original colour.

Here is the codesandbox with this basic setup. Here you will see the correct behaviour in the development build.

I have also deployed this code directly from the codesandbox via Netlify: https://csb-m5c9z.netlify.app/#/users/create. Here you will see the incorrect behaviour in the production build.

Looking at the css in the production build, you can see that the color from react-admin styles jss91 is overwritten by the MuiTab-TextColorInherit class.

image

Looking at how they are included in the final build, the MuiTab styles are loaded after the RaTabbedForm styles, meaning they will get preference.

image

So I think this may be related to how the CSS is imported or loaded.

update

Everything mentioned above is still valid in order to reproduce the issue. I updated to react-admin v3.13.4 in the codesandbox and did a new deployment so you can see the production build behaviour.

Let me know if I should check or change something else.