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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src-docs/src/views/suggest_item/suggest_item_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.

<EuiCode>label</EuiCode> will have a fixed width and use ellipsis
whenever its content is too long. It is possible to show the full text
by setting <EuiCode>layout</EuiCode> to <EuiCode>inline</EuiCode>.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`EuiSuggestItem is rendered 1`] = `
<div
aria-label="aria-label"
class="euiSuggestItem testClass1 testClass2"
data-test-subj="test subject string"
>
<span
class="euiSuggestItem__type euiSuggestItem__type--primary"
>
<svg
class="euiIcon euiIcon--medium euiIcon--primary euiIcon-isLoading"
focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
/>
</span>
<span
class="euiSuggestItem__label euiSuggestItem__layout--set"
/>
<span
class="euiSuggestItem__description"
/>
</div>
`;
5 changes: 3 additions & 2 deletions src/components/suggest_item/_suggest_item.scss
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ $buttonTypes: (
font-size: $euiFontSizeXS;
// sass-lint:disable-block no-trailing-whitespace
white-space: nowrap;

@each $name, $color in $buttonTypes {
.euiSuggestItem__type--#{$name} {
background-color: tintOrShade($color, 90%, 50%);
Expand Down Expand Up @@ -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

overflow: hidden;
text-overflow: ellipsis;
display: block;
Expand Down
9 changes: 5 additions & 4 deletions src/components/suggest_item/suggest_item.js
Original file line number Diff line number Diff line change
@@ -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.

import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import { EuiIcon, IconPropType, IconColor, IconType } from '../icon';
import { TypePredicateKind } from 'typescript';
import { EuiIcon, IconPropType } from '../icon';

const colorToClassNameMap = {
primary: 'euiSuggestItem__type--primary',
Expand Down Expand Up @@ -65,7 +63,10 @@ EuiSuggestItem.propTypes = {
/**
* Takes 'icon' for EuiIcon and 'color'. 'color' can be either our palette colors (primary, secondary, etc) or a hex value.
*/
type: PropTypes.object,
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.

/**
* Label for suggestion
*/
Expand Down
7 changes: 6 additions & 1 deletion src/components/suggest_item/suggest_item.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.

icon: 'search',
color: 'primary',
};

describe('EuiSuggestItem', () => {
test('is rendered', () => {
const component = render(<EuiSuggestItem {...requiredProps} />);
const component = render(<EuiSuggestItem {...requiredProps} type={TYPE} />);

expect(component).toMatchSnapshot();
});
Expand Down