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

Make description prop optional for EuiDescribedFormGroup. #1191

Conversation

cjcenizal
Copy link
Contributor

Summary

Sometimes the title speaks for itself 😄

Checklist

  • Documentation examples were added
  • A changelog entry exists and is marked appropriately
  • Jest tests were updated or added to match the most common scenarios
  • This was checked against keyboard-only and screenreader scenarios

@cjcenizal cjcenizal force-pushed the improvement/described-form-group-description branch from 293e7b8 to 8c4e5d3 Compare September 13, 2018 18:57
@cjcenizal cjcenizal force-pushed the improvement/described-form-group-description branch from 8c4e5d3 to a63e8ad Compare September 13, 2018 18:58
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; code review, pulled and tested doc example

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to approve this for the flexibility it affords, but I think the spacing looks really odd without a description and we will need to pay attention to usages that don't supply one.

Thanks for the PR!

@cjcenizal cjcenizal merged commit 0836392 into elastic:master Sep 13, 2018
@cjcenizal cjcenizal deleted the improvement/described-form-group-description branch September 13, 2018 20:02
@snide snide mentioned this pull request Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants