[DRAFT] Allowing EuiFormRow props on EuiFieldText #3837
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
|
This seems pretty straightforward and works well! Migrating to this as EUI's recommended approach I think would pay big dividends down the line 👍 👍 |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
cchaos
changed the title
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
# Conflicts: # src-docs/src/services/playground/_playground_compiler.scss # src-docs/src/services/playground/knobs.js # src-docs/src/services/playground/playground.js # src-docs/src/views/button/playground.js # src-docs/src/views/form_controls/playground.js # src-docs/src/views/guidelines/writing.js
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
cchaos
changed the title
| const { finalNodes: prependNodes, finalNodeIDs: prependIDs } = renderSideNode( | ||
| 'prepend', | ||
| id, | ||
| prepend | ||
| ); | ||
| const { finalNodes: appendNodes, finalNodeIDs: appendIDs } = renderSideNode( | ||
| 'append', | ||
| id, | ||
| append | ||
| ); |
There was a problem hiding this comment.
Sorry for all the extra individual comment pings – I forgot how to use GitHub for a bit...
Just did an a11y focused review and this looks awesome – a great improvement for text fields and a great pattern to continue to emulate for other (more complicated) form controls as well.
Will do another code review focusing on the code bits. @thompsongl might like to as well.
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
|
👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment. |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
# Conflicts: # src-docs/src/views/flyout/flyout_max_width.js # src-docs/src/views/form_compressed/form_compressed.js # src-docs/src/views/form_compressed/form_horizontal.js # src-docs/src/views/form_compressed/form_horizontal_help.js # src-docs/src/views/form_layouts/described_form_group.js # src-docs/src/views/form_validation/validation.js # src-docs/src/views/i18n/context.js # src-docs/src/views/i18n/i18n_attribute.js # src/components/form/field_text/field_text.tsx # src/components/form/form_row/form_row.tsx
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
|
👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment. |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3837/ |
|
👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment. |
|
❌ We're automatically closing this PR due to lack of activity. Please comment if you feel this was done in error. |
Ref #2493
This is just a quick POC ofEstablishing a pattern of how we can extend EuiFormRow props on each input field component so that they can personally handle the correct values to pass down to EuiFormRow.This doesn't explicitly "fix" all the issues from the referenced ticket, but it just showcase my idea of how easy it could be to not break current implementations but allow people to convert to the new format.