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

[ML] Data frames: Fixes table sorting. #43859

Merged
merged 3 commits into from
Aug 27, 2019
Merged

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Aug 23, 2019

Summary

  • EuiInMemoryTable will not correctly reflect prop updates like sorting. So for example, when the component gets mounted with sorting={false} it will never consider a later update to make sorting available after all data is loaded. This PR fixes it by mounting the component only once sorting was set properly. This affected all data frame analytics/transform tables.
  • This consolidates code where we had multiple custom type definitions for EuiInMemoryTable because it's not based on TypeScript itself yet. The PR adds TypeScript Prop definitions for
    the component in ml/common/types/eui/in_memory_table.ts based on React propTypes and exposes a MlInMemoryTable component that wraps EuiInMemoryTable. I'll be in contact with the EUI team so they can make use of this for EUI itself.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@walterra walterra added bug Fixes for quality problems that affect the customer experience release_note:fix :ml v8.0.0 Feature:Transforms ML transforms v7.4.0 Feature:Data Frame Analytics ML data frame analytics features labels Aug 23, 2019
@walterra walterra requested a review from a team as a code owner August 23, 2019 14:01
@walterra walterra self-assigned this Aug 23, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

Tested this locally. LGTM ⚡️

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

LGTM

@walterra walterra merged commit 0772e19 into elastic:master Aug 27, 2019
@walterra walterra deleted the ml-eui-types branch August 27, 2019 09:23
walterra added a commit to walterra/kibana that referenced this pull request Aug 27, 2019
- EuiInMemoryTable will not correctly reflect prop updates like sorting. So for example, when the component gets mounted with sorting={false} it will never consider a later update to make sorting available after all data is loaded. This PR fixes it by mounting the component only once sorting was set properly. This affected all data frame analytics/transform tables.
- This consolidates code where we had multiple custom type definitions for EuiInMemoryTable because it's not based on TypeScript itself yet. The PR adds TypeScript Prop definitions for
the component in ml/common/types/eui/in_memory_table.ts based on React propTypes and exposes a MlInMemoryTable component that wraps EuiInMemoryTable. I'll be in contact with the EUI team so they can make use of this for EUI itself.
walterra added a commit that referenced this pull request Aug 27, 2019
- EuiInMemoryTable will not correctly reflect prop updates like sorting. So for example, when the component gets mounted with sorting={false} it will never consider a later update to make sorting available after all data is loaded. This PR fixes it by mounting the component only once sorting was set properly. This affected all data frame analytics/transform tables.
- This consolidates code where we had multiple custom type definitions for EuiInMemoryTable because it's not based on TypeScript itself yet. The PR adds TypeScript Prop definitions for
the component in ml/common/types/eui/in_memory_table.ts based on React propTypes and exposes a MlInMemoryTable component that wraps EuiInMemoryTable. I'll be in contact with the EUI team so they can make use of this for EUI itself.
simianhacker pushed a commit to simianhacker/kibana that referenced this pull request Aug 27, 2019
- EuiInMemoryTable will not correctly reflect prop updates like sorting. So for example, when the component gets mounted with sorting={false} it will never consider a later update to make sorting available after all data is loaded. This PR fixes it by mounting the component only once sorting was set properly. This affected all data frame analytics/transform tables.
- This consolidates code where we had multiple custom type definitions for EuiInMemoryTable because it's not based on TypeScript itself yet. The PR adds TypeScript Prop definitions for
the component in ml/common/types/eui/in_memory_table.ts based on React propTypes and exposes a MlInMemoryTable component that wraps EuiInMemoryTable. I'll be in contact with the EUI team so they can make use of this for EUI itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Data Frame Analytics ML data frame analytics features Feature:Transforms ML transforms :ml release_note:fix v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants