Skip to content

EuiSuggestItems tests pass#1

Merged
andreadelrio merged 1 commit intoandreadelrio:suggest-itemfrom
thompsongl:suggest-item
Jul 3, 2019
Merged

EuiSuggestItems tests pass#1
andreadelrio merged 1 commit intoandreadelrio:suggest-itemfrom
thompsongl:suggest-item

Conversation

@thompsongl
Copy link
Copy Markdown

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

text: (
<p>
By default <EuiCode>EuiSuggestItem</EuiCode>'s{' '}
By default <EuiCode>EuiSuggestItem</EuiCode>&apos;s{' '}
Copy link
Copy Markdown
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.


.euiSuggestItem__description, .euiSuggestItem__label {
.euiSuggestItem__description,
.euiSuggestItem__label {
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.


import { EuiSuggestItem } from './suggest_item';

const TYPE = {
Copy link
Copy Markdown
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 <549577+cchaos@users.noreply.github.com>
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