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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specification of id prop on ErrorMessage #2427

Closed
krichter722 opened this issue Apr 19, 2020 · 11 comments
Closed

Allow specification of id prop on ErrorMessage #2427

krichter722 opened this issue Apr 19, 2020 · 11 comments

Comments

@krichter722
Copy link

krichter722 commented Apr 19, 2020

馃殌 Feature request

Current Behavior

The type ErrorMessage doesn't have an id property.

Desired Behavior

It'd be nice id?: string would be added to the type ErrorMessage and set on the outermost component that serves as error message.

Suggested Solution

Add id={this.props.id} to the outer component in ErrorMessage.

Who does this impact? Who is this for?

This improves handling of error messages in e2e tests. It doesn't impact users in any way as the prop can still be omitted.

Describe alternatives you've considered

Wrapping ErrorMessage in a div which has the sole purpose of providing the id which is not nice.

Additional context

./.

@lakbychance
Copy link

lakbychance commented May 1, 2020

Let's say we have <ErrorMessage id="sample-id"/> after suggested change.(Ignore other props for now)
There are three ways the outer component can serve as an error message in ErrorMessage (as per master code):-

  1. Using render props, so the id given to ErrorMessage can't be used inside render props.
  2. Using children function, so the id given to ErrorMessage can't be used inside the children function.
  3. Using component prop of ErrorMessage, so the id given to ErrorMessage will work because of the way it's implemented in the code.

So giving ErrorMessage an id prop will work for 1 out of 3 scenarios.
I might be wrong here but let me know if this interpretation of suggested change was valid or not.

@DouglasPds
Copy link

I will try to implement this.

@DouglasPds
Copy link

The id passed to ErrorMessage need to be rendered inside the element in children {msg => <div>{msg}</div>}, and in element in render function render={msg => <div>{msg}</div>} ??

@zeotuan
Copy link

zeotuan commented Jul 16, 2021

Hi i want to take on this issue. If no one is working on it.

@imadityadi
Copy link

imadityadi commented Sep 1, 2021

Is anyone working on this ??

@Rajat-patel19
Copy link

Hey , I am a beginner caan anyone help me out .

ksat8384 added a commit to ksat8384/formik that referenced this issue Nov 26, 2021
Added an id prop to ErrorMessage [jaredpalmer#2427]
@ZainGulbaz
Copy link

@krichter722 Can you please assign this to me? Thanks.

@krichter722
Copy link
Author

@ZainGulbaz No, I can't assign users to issues. There seem to be no assignments to issues in this project.

@ZainGulbaz
Copy link

@krichter722 Thanks for the clarification. I will make a pull request then.

@udaysai1311
Copy link

Can I work on this?

@krichter722
Copy link
Author

A property id?: string has been added to ErrorMessageProps in #3825. Afaik the id is passed to the DOM element rendered by ErrorMessage if render and children are not specified, according to this spaghetti code:

    return !!touch && !!error
      ? render
        ? isFunction(render)
          ? render(error)
          : null
        : children
        ? isFunction(children)
          ? children(error)
          : null
        : component
        ? React.createElement(component, rest as any, error)
        : error
      : null;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants