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

Cannot use SUID (Material UI) components #27

Closed
NightmanCZ90 opened this issue Mar 12, 2023 · 6 comments
Closed

Cannot use SUID (Material UI) components #27

NightmanCZ90 opened this issue Mar 12, 2023 · 6 comments
Assignees
Labels
enhancement New feature or request question Further information is requested

Comments

@NightmanCZ90
Copy link

Hello, I recently started using solidjs for my project. I used SUID (Material UI for SolidJs) and wanted to use this library for form validations. When I pass regular input in Form component, everything works as expected. But when I use TextField from SUID instead, I cannot submit the form as the form has no values filled out. It took me couple of hours to debug this issue. It is due to SUID having different event value from onChange and onInput that this library needs. Is there any elegant workaround (using Typescript) to pass this issue? I have an working example code, but it gives me couple of typescript warnings and Im not sure, if overwriting some values from target and currentTarget will not mess up anything else.

      <Form of={loginForm} onSubmit={handleSubmit}>
          <Field
            of={loginForm}
            name="email"
          >
            {(field) =>
                <TextField
                  {...field.props}
                  type="email"
                  // Reassign onInput and onChange, overwrite events
                  onInput={(e) => field.props.onInput({ ...e, currentTarget: { ...e.currentTarget, value: e.target.value } }) }
                  onChange={(e) => field.props.onInput(e)}
                  variant="outlined"
                  component="input"
                  label="E-mail"
                  required
                  error={Boolean(field.error)}
                  helperText={field.error}
                  value={field.value || ''}
                />}
          </Field>
          <div class="signin-form--button">
            <Button
              type="submit"
              fullWidth
              color="secondary"
              variant="contained"
            >
              Sign in
            </Button>
          </div>
        </Form>
@fabian-hiller fabian-hiller self-assigned this Mar 13, 2023
@fabian-hiller fabian-hiller added the enhancement New feature or request label Mar 13, 2023
@fabian-hiller
Copy link
Owner

Thank you for creating the issue. I remember testing SUID with Modular Forms a few months ago. At that time there were no problems. I will have a look at it and give you feedback by tomorrow.

@fabian-hiller
Copy link
Owner

I don't know much about SUID or MUI component library, but took a look at the code on GitHub and suspect the following problem. The event listener that can be passed to the TextField component via the onInput prop is not added to the <input /> element but to an outer <div /> element. For this reason, the value can only be retrieved via event.target and not via event.currentTarget.

For Modular Forms it is important that the field.props are passed directly to the <input /> element, because this is relevant for several functionalities of the library. For example, focusing an element on invalid input.

I therefore see the problem on the side of SUID. I tried via inputProps and InputProps to pass the field.props directly to the <input /> element. Unfortunately without success. I recommend you to create an issue at SUID and exchange with @swordev. If needed, feel free to link me and this issue.

@fabian-hiller fabian-hiller added the question Further information is requested label Mar 14, 2023
@fabian-hiller
Copy link
Owner

Is the problem still present? Should I create an issue at SUID?

@NightmanCZ90
Copy link
Author

I haven't really contacted them. I checked their source code for inputs and from my understanding (although i could be wrong) it seems, they are merging those two event handlers (onChange and onInput) together.

I tested it and it seems that the only way I could work around it is to remap the field.props for TextField.
So far I have used this workaround below.

import { FieldElement } from "@modular-forms/solid";
import { JSX } from "solid-js";

export const remapFieldProps = (fieldProps: {
  name: string;
  ref: (element: FieldElement) => void;
  onInput: JSX.EventHandler<FieldElement, InputEvent>;
  onChange: JSX.EventHandler<FieldElement, Event>;
  onBlur: JSX.EventHandler<FieldElement, FocusEvent>;
}) => ({
  name: fieldProps.name,
  inputRef: fieldProps.ref,
  onBlur: fieldProps.onBlur,
  onChange: (e: any) => { fieldProps.onChange(e); fieldProps.onInput(e) }
});

=====================
<Field
  of={loginForm}
  name="email"
  validate={[
     required('Please enter your email.'),
     email('Please enter a valid email address.')
  ]}
>
  {(field) =>
    <TextField
      inputProps={{ ...remapFieldProps(field.props) }}
      type="email"
      value={field.value || ''}
      error={Boolean(field.error)}
      helperText={field.error}
    />}
</Field>

So far I haven't found any issue using this approach. But it may yet still come.

@NightmanCZ90
Copy link
Author

I will contant them and link this issue.

@fabian-hiller
Copy link
Owner

According to current knowledge, it looks like it is a "problem" with the naming on the part of SUID. Here is a workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants