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

[EuiCodeEditor] does not work well with EuiFormRow to assign a label #2961

Closed
myasonik opened this issue Mar 2, 2020 · 8 comments · Fixed by #3212
Closed

[EuiCodeEditor] does not work well with EuiFormRow to assign a label #2961

myasonik opened this issue Mar 2, 2020 · 8 comments · Fixed by #3212

Comments

@myasonik
Copy link
Contributor

myasonik commented Mar 2, 2020

When using EuiCodeEditor inside of EuiFormRow there is no convenient way to assign a an id to the textarea.

In places Kibana has started to shim this after render but inconsistently and in Kibana probably isn't the best place to do this anyway.

Example of what I mean: editor_example.tsx#L34-L45

I'm not sure what the best solution is here because I know EuiCodeEditor has its hands tied by react-ace a little. Will probably need to explore if the best solution can be implemented in EuiCodeEditor or in EuiFormRow.

@anishagg17
Copy link
Contributor

anishagg17 commented Mar 3, 2020

@myasonik i am working on it and will soon update you

@anishagg17
Copy link
Contributor

currently reacct-ace doesn't has an id prop that's why this problem is occurring I have raised an issue in react-ace library to fix this .

@myasonik
Copy link
Contributor Author

myasonik commented Mar 4, 2020

In the mean time, can we do a fix similar to what was done in kibana? After render, query the dom for a textarea, then apply an id to that.

@thompsongl @chandlerprall does that sound like an all right interim solution? (Given that we don't know when/if react-ace will fix their hole and then when/if we'll update after that)

@chandlerprall
Copy link
Contributor

chandlerprall commented Mar 4, 2020

Taking this a little further, we could support any additional attributes (limited to id + aria stuff, maybe?) with an approach similar to

keysOf(rest).forEach(key => {
if (typeof rest[key] !== 'string') {
throw new Error(
`Unhandled property type. EuiOverlayMask property ${key} is not a string.`
);
}
this.overlayMaskNode!.setAttribute(key, rest[key]!);
});

i.e. have EuiCodeEditor accept an additional prop, an object with whatever attributes we want to support, and set those attribute/values after mount

@anishagg17
Copy link
Contributor

@myasonik do i need to apply a post-render update for this?

@myasonik
Copy link
Contributor Author

Yes, this issue and #2960 should be fixed with the solution @chandlerprall posted above.

Make a new object prop called textareaProps.
After render of the component, apply any props from textareaProps to the textarea rendered.

@cchaos cchaos changed the title EuiCodeEditor does not work well with EuiFormRow to assign a label [EuiCodeEditor] does not work well with EuiFormRow to assign a label Mar 18, 2020
@anishagg17
Copy link
Contributor

Working on this would update shortly

@anishagg17
Copy link
Contributor

Screenshot 2020-04-01 at 7 48 09 PM

@myasonik this technique has solved the issue, I will make a pr when one of my or gets closed

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

Successfully merging a pull request may close this issue.

3 participants