The Wayback Machine - https://web.archive.org/web/20201124120500/https://github.com/formium/formik/issues/2427
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

Open
krichter722 opened this issue Apr 19, 2020 · 6 comments
Open

Allow specification of id prop on ErrorMessage #2427

krichter722 opened this issue Apr 19, 2020 · 6 comments

Comments

@krichter722
Copy link

@krichter722 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

./.

@lapstjup
Copy link

@lapstjup lapstjup 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.