Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-render withStyle HOC on redux action issue #16029

Closed
sashberd opened this issue Jun 3, 2019 · 13 comments
Closed

Re-render withStyle HOC on redux action issue #16029

sashberd opened this issue Jun 3, 2019 · 13 comments
Labels
status: waiting for author Issue with insufficient information

Comments

@sashberd
Copy link

sashberd commented Jun 3, 2019

Hi,

I am now upgrading my application to MUI v3.9.3. We are using withStyles HOC in our upgrade. Our application supports both RTL and LTS languages

We have a switcher languages button that is activate redux action for RTL or LTR prop.
The issue is that all UI switches to new direction, but classes from withStyles are not. On page reload all becomes good.

The major issue we have with TextFields (floating) label that is not align by RTL. I found this issue #9332 that is explains about jss-rtl plugin, that was installed in our application from the start of MUI upgrade. Moreover, our RTL-LTR switch is working well besides issue below.

After some searching I suppose that the issue is only with MuiInputLabel-shrink class, transform-origin css property, which is not updates on direction switch.

Below attached example from application when I switched from RTL to LTR and (floating) labels visibility issue:
Screen Shot 2019-06-03 at 16 00 52
Screen Shot 2019-06-03 at 16 01 06

In that specific example If the transform-origin css property changed to top-left, every thing become good (The value of the css property become good after reload)

If there is missing some information i this issue will be glad to provide it

@sashberd sashberd changed the title Re-render withStyle HOC on redux action Re-render withStyle HOC on redux action issue Jun 3, 2019
@eps1lon
Copy link
Member

eps1lon commented Jun 3, 2019

I think this applies to any component that does some layout computations. In general I would just remount the whole app when switching direction. This should solve most of the subtle issues with regards to layout.

@sashberd
Copy link
Author

sashberd commented Jun 3, 2019

@eps1lon Remount application is the possible solution, but why to do that if until now we found only this issue. If the problem is in jss-rtl I will address this issue to their thread.

@eps1lon
Copy link
Member

eps1lon commented Jun 3, 2019

Could you share your full example? We have an RTL switch in our docs as well and no issues with label positioning. Did you follow our right-to-left guide?

@eps1lon eps1lon added the status: waiting for author Issue with insufficient information label Jun 3, 2019
@sashberd
Copy link
Author

sashberd commented Jun 3, 2019

@eps1lon As I said, We did rtl-guide and install jss-rtl plugin (this was the first step in our upgrade)

import {
  withTheme,
  TextField as TextFieldNG,
  withStyles,
  Button as ButtonNG,
} from '@material-ui/core';

This is our styles const

const giftCardRecipientDetailsStepStyles = ({ axisPalette }) => {
  return {
    textFieldRoot: {
      '&:after': {
        borderBottom: `2px solid ${axisPalette.accentColor}`,
      },
    },
    error: {},
    focused: {},
    disabled: {},
    textFieldLabelRoot: {
      '&$focused': {
        color: axisPalette.accentColor,
      },
      '&$error': {
        color: axisPalette.generalInput.inputGreyedOut,
      },
      fontSize: '16px',
      right: 0,
    },
    textFieldUnderline: {
      '&:hover:not($disabled):not($focused):not($error):before': {
        borderBottom: `1px solid ${axisPalette.accentTextColor}`,
      },
    },
    rewardTextareaRoot: {
      backgroundColor: 'rgba(214, 214, 214, 0.4)',
      borderRadius: '2px',
      textIndent: '5px',
      minHeight: '95px',
    },
    rewardTextareaLabel: {
      padding: '6px',
      '&$error': {
        color: axisPalette.generalInput.inputGreyedOut,
      },
    },
    rewardTextareaUnderline: {
      '&:before': {
        content: 'none',
      },
    },
    rewardTextAreaInputRoot: {
      fontSize: '16px',
      minHeight: '75px',
      '&:after': {
        borderBottom: 'none',
        display: 'none',
      },
    },
    textAreaHelperTextRoot: {
      marginTop: '12px',
    },
    rewardTextareaInputBasedMultiline: {
      paddingBottom: 0,
    },
    rewardTextareaInputBasedInputMultiline: {
      overflow: 'hidden',
    },
    empty: {},
  };
};

This is out input renderer function:

renderInput = ({ meta: { touched, error } = {}, input: { ...inputProps }, ...props }) => {
    // return <TextField errorText={touched && error} {...inputProps} {...props} fullWidth />;

    const { classesinstance } = props;

    return (
      <TextFieldNG
        error={touched && error}
        helperText={touched && error}
        {...inputProps}
        {...props}
        fullWidth
        InputLabelProps={{
          FormLabelClasses: {
            root: classesinstance.textFieldLabelRoot,
            focused: classesinstance.focused,
            error: classesinstance.error,
          },
        }}
        InputProps={{
          classes: {
            root: props.multiline
              ? classesinstance.rewardTextAreaInputRoot
              : classesinstance.textFieldRoot,
            underline: props.multiline
              ? classesinstance.rewardTextareaUnderline
              : classesinstance.textFieldUnderline,
            multiline: props.multiline
              ? classesinstance.rewardTextareaInputBasedMultiline
              : classesinstance.empty,
            inputMultiline: props.multiline
              ? classesinstance.rewardTextareaInputBasedInputMultiline
              : classesinstance.empty,
          },
        }}
      />
    );
  };

This render method:

<form onSubmit={handleSubmit(this.handleOnSuccess)}>
         <Field
           type="text"
           label={t('giftcardHolderName')}
           name="giftcardHolderName"
           component={this.renderInput}
           classesinstance={classes}
         />
<div className="giftcard-greeting">
           <Field
             name="giftcardGreeting"
             label={t('giftcardGreeting')}
             component={this.renderInput}
             multiline
             classes={{ root: classes.rewardTextareaRoot, label: classes.rewardTextareaLabel }}
             classesinstance={classes}
             FormHelperTextProps={{ classes: { root: classes.textAreaHelperTextRoot } }}
           />
         </div>
</form

This is class HOC export:

const enhance = compose(
  translate(),
  //muiThemeable(),
  withTheme(),
  withStyles(giftCardRecipientDetailsStepStyles, { withTheme: true }),
  reduxForm({
    form: 'GiftCardForm',
    validate: (values, props) => {
      const errors = {};
      const { t, giftCardAmounts } = props;

      ...Form validatons

      return errors;
    },
  })
);

export default enhance(GiftCardRecipientDetailsStep);

@eps1lon Do you need also redux code here?

@eps1lon
Copy link
Member

eps1lon commented Jun 3, 2019

What about setting the dir attribute?

@sashberd
Copy link
Author

sashberd commented Jun 4, 2019

@sashberd Yes we did the whole rtl guide as it is written in MUI page:

This is html direction when the problem
Screen Shot 2019-06-04 at 9 25 16
Screen Shot 2019-06-04 at 9 30 30

is exists:

and this is jss creation from context object:
Screen Shot 2019-06-04 at 9 29 33

Here the context creation code:

import { SheetsRegistry } from 'jss';
import { createMuiTheme, createGenerateClassName } from '@material-ui/core/styles';

function generateTheme(theme, userAgent, isRtl) {
  return createMuiTheme({ userAgent, ...theme, direction: isRtl ? 'rtl' : 'ltr' });
}

function createPageContext(theme, userAgent, isRtl) {
  return {
    theme: generateTheme(theme, userAgent, isRtl),
    // This is needed in order to deduplicate the injection of CSS in the page.
    sheetsManager: new Map(),
    // This is needed in order to inject the critical CSS.
    sheetsRegistry: new SheetsRegistry(),
    // The standard class name generator.
    generateClassName: createGenerateClassName(),
  };
}

let pageContext;

export default function getPageContext(theme, userAgent, isRtl) {
  // Make sure to create a new context for every server-side request so that data
  // isn't shared between connections (which would be bad).
  if (!process.browser) {
    return createPageContext(theme, userAgent, isRtl);
  }

  // Reuse context on the client-side.
  if (!pageContext) {
    pageContext = createPageContext(theme, userAgent, isRtl);
  }
  return pageContext;
}

@eps1lon
Copy link
Member

eps1lon commented Jun 4, 2019

Could you put together a clonable repository? It's very hard to debug CSS issues without devtools.

@sashberd
Copy link
Author

sashberd commented Jun 4, 2019

@eps1lon I cannot provide to you a fork to repo cause it is not my private, but organization . I could try provide to you access to our stg env to illustrate you the issue online

@eps1lon
Copy link
Member

eps1lon commented Jun 4, 2019

No I mean some isolated, minimal example. This helps a lot tracking down the issue.

@sashberd
Copy link
Author

sashberd commented Jun 4, 2019

@eps1lon Sorry I do not have such option. This specific component is not stand alone and has multiple dependencies from server, env etc. Thus, this is not possible

@sashberd
Copy link
Author

@oliviertassinari You close issue that was not resolved and the problem exists now in material ui... I cannot provide to you code, but I provided enough information in order to reproduce this issue in yours engines. So why close and not resolve?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 10, 2019

@sashberd We ask our users to provide live reproductions for two reasons:

  1. Somebody not willing to take the time to provide a reproduction signal that the problem is not really important for them. It helps us only working on important enough problems.
  2. Somebody providing a reproduction save us time. It helps us solve more problems.

@omerpeled
Copy link

same with Switch component

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting for author Issue with insufficient information
Projects
None yet
Development

No branches or pull requests

4 participants