Skip to content

Conversation

@drinchev
Copy link
Contributor

No description provided.

@drinchev drinchev requested a review from blumendorf November 19, 2019 08:16
});
const itemFoo = getByText('Item Foo');
const itemBar = getByText('Item Bar');
expect(itemFoo.classList.contains('is-active')).toBeTruthy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be better to use .toBe(true) if you are expecting this to actually be true?

const itemFoo = getByText('Item Foo');
const itemBar = getByText('Item Bar');
expect(itemFoo.classList.contains('is-active')).toBeTruthy();
expect(itemBar.classList.contains('is-active')).toBeFalsy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for .toBe(false)


```jsx
import { InputList, InputListItem } from '@lightelligence/react';
const onClick = (value) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const onClick = (value) => {
const onChange = (value) => {

});

const component = getByText('Component');
expect(component.classList.contains('myClass')).toBeTruthy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
expect(component.classList.contains('myClass')).toBeTruthy();
expect(component.classList.contains('myClass')).toBe(true);

{selectedElement.props.children}
</div>
)}
<div className={classnames(olt.V2DropdownContent)}>
Copy link
Contributor

Choose a reason for hiding this comment

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

The dropdown content is rendered behind the code field in the guide.

],
});
const component = getByTestId('component');
expect(component.classList.contains('myClass')).toBeTruthy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
expect(component.classList.contains('myClass')).toBeTruthy();
expect(component.classList.contains('myClass')).toBe(true);

});
const component = getByTestId('component');
fireEvent.click(component);
expect(component.classList.contains('is-open')).toBeTruthy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
expect(component.classList.contains('is-open')).toBeTruthy();
expect(component.classList.contains('is-open')).toBe(true);

fireEvent.click(component);
expect(component.classList.contains('is-open')).toBeTruthy();
fireEvent.click(component);
expect(component.classList.contains('is-open')).toBeFalsy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
expect(component.classList.contains('is-open')).toBeFalsy();
expect(component.classList.contains('is-open')).toBe(false);

);
const selectedElement = selectedChild && React.cloneElement(selectedChild);

return (
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a couple of components that allow passing props to their children (e.g. Card with contentProps, actionProps, headerProps, editProps, ...) should we allow that consistently for all components?

@drinchev drinchev requested a review from blumendorf November 19, 2019 10:08
@drinchev drinchev merged commit f46d413 into alpha Nov 19, 2019
@drinchev drinchev deleted the feat/v2dropdown branch November 20, 2019 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants