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

EuiSuggestItems tests pass #1

Merged
merged 1 commit into from
Jul 3, 2019

Conversation

thompsongl
Copy link

A couple linting things and one React thing to make sure EuiSuggestItem is always provided the data it expects (type)

@@ -77,7 +77,7 @@ export const SuggestItemExample = {
],
text: (
<p>
By default <EuiCode>EuiSuggestItem</EuiCode>'s{' '}
By default <EuiCode>EuiSuggestItem</EuiCode>&apos;s{' '}
Copy link
Author

Choose a reason for hiding this comment

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

Writing text in the docs is weird sometimes. This just uses the unicode code for apostrophe so we don't have to escape it.

@@ -69,7 +69,8 @@ $buttonTypes: (
}
}

.euiSuggestItem__description, .euiSuggestItem__label {
.euiSuggestItem__description,
.euiSuggestItem__label {
Copy link
Author

Choose a reason for hiding this comment

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

Our linter wants each selector on its own line

@@ -1,9 +1,7 @@
/* eslint-disable @typescript-eslint/no-unused-vars */
Copy link
Author

Choose a reason for hiding this comment

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

No need for TypeScript things yet. I'll help you convert when the time comes.

type: PropTypes.shape({
icon: IconPropType,
color: PropTypes.oneOfType([PropTypes.oneOf(COLORS), PropTypes.string]),
}).isRequired,
Copy link
Author

Choose a reason for hiding this comment

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

Being more specific about what type wants, and making it required so it's always available. We can provide default values if that seems more like how people will use it.

@@ -4,9 +4,14 @@ import { requiredProps } from '../../test/required_props';

import { EuiSuggestItem } from './suggest_item';

const TYPE = {
Copy link
Author

Choose a reason for hiding this comment

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

Because we made type required, we need to add it to the test so it doesn't fail.

@andreadelrio andreadelrio merged commit 147487e into andreadelrio:suggest-item Jul 3, 2019
andreadelrio added a commit that referenced this pull request Jul 15, 2019
andreadelrio pushed a commit that referenced this pull request Aug 28, 2019
Update defazio fork to 13.0
andreadelrio pushed a commit that referenced this pull request Feb 24, 2020
* removed duplicate icons

* Fixing changelog and updating tests (#1)

Co-authored-by: Caroline Horn <[email protected]>
andreadelrio added a commit that referenced this pull request May 29, 2020
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.

2 participants