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

fix(react-grid): correct changing DataTypeProvider.for property in runtime #2201

Merged
merged 7 commits into from
Aug 1, 2019
Merged

fix(react-grid): correct changing DataTypeProvider.for property in runtime #2201

merged 7 commits into from
Aug 1, 2019

Conversation

LazyLahtak
Copy link
Contributor

@LazyLahtak LazyLahtak commented Jul 29, 2019

Fixes #2047

Andrey Ignatovskiy and others added 2 commits July 29, 2019 17:08
@LazyLahtak LazyLahtak marked this pull request as ready for review July 30, 2019 10:18
@LazyLahtak LazyLahtak changed the title fit(react-grid): correct changing DataTypeProvider.for property in runtime fix(react-grid): correct changing DataTypeProvider.for property in runtime Jul 30, 2019
@LazyLahtak
Copy link
Contributor Author

FPR

<DataTypeProvider
for={['test']}
getAvailableFilterOperations={() => {}}
/>
{pluginDepsToComponents({})}
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is unnecessary

constructor(props) {
super(props);
this.state = {
columnNames: [`${defaultDeps.getter.tableColumns[0].column.name}`],
Copy link
Contributor

Choose a reason for hiding this comment

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

``${` is redundant here, use simple quotes instead

<Template name="table">
<div>
<TemplateConnector>
{({ tableColumns }) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

divs are redundant

const defaultDeps = {
getter: {
tableColumns: [
{ key: `${TABLE_DATA_TYPE}_a`, type: TABLE_DATA_TYPE, column: { name: 'a' } },
Copy link
Contributor

Choose a reason for hiding this comment

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

Just key: 'a' is enough

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need type here. Besides that, TABLE_DATA_TYPE is undefined since grid-core is mocked

</PluginHost>
));
expect(getComputedState(tree).getAvailableFilterOperations)
.toEqual(expect.any(Function));
});

it('should re-register templates when "for" property changed in runtime', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need a similar test against valueEditor


const tree = mount(<Test />);

expect(tree
Copy link
Contributor

Choose a reason for hiding this comment

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

Simplify this: just check a formatterComponent's props

@LazyLahtak
Copy link
Contributor Author

FPR

@LazyLahtak LazyLahtak requested a review from ushkal July 30, 2019 12:45

it('should re-register valueFormatter templates', () => {
const DataFormatter = () => null;
const tree = mount(<Test
Copy link
Contributor

Choose a reason for hiding this comment

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

We use another formatting:

mount((
  <Test ... />
))

@LazyLahtak LazyLahtak requested a review from ushkal July 31, 2019 12:13
@LazyLahtak
Copy link
Contributor Author

FPR

@churkin churkin merged commit 3732635 into DevExpress:master Aug 1, 2019
@LazyLahtak LazyLahtak deleted the data-type-provider-bug-for branch August 1, 2019 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants