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.

Sign up for GitHub

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

Jump to bottom

Convert EuiSearchBar to TypeScript #2909

Merged
merged 15 commits into from
Mar 6, 2020
Merged

Convert EuiSearchBar to TypeScript #2909

merged 15 commits into from
Mar 6, 2020

Conversation

pugnascotia
Copy link
Contributor

Convert EuiSearchBar to TypeScript.

Also:

  • Add a .ignore file so that ripgrep ignores the docs bundle.
  • Fix type:whatever searches in the docs
  • Fix grammatical errors and an NPE in the docs

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2909/

@pugnascotia
Copy link
Contributor Author

I've merged in master. Any chance of a review?

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2909/

chandlerprall
chandlerprall suggested changes Feb 28, 2020
View reviewed changes
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Thanks again for taking this on! ❤️

Some questions, couple definite suggestions.

src/components/basic_table/in_memory_table.tsx Show resolved Hide resolved
src/components/basic_table/in_memory_table.tsx Show resolved Hide resolved
src/components/basic_table/in_memory_table.tsx Show resolved Hide resolved
src/components/search_bar/query/ast_to_es_query_dsl.ts Outdated Show resolved Hide resolved
src/components/search_bar/query/ast_to_es_query_dsl.ts Outdated Show resolved Hide resolved
.ignore Outdated Show resolved Hide resolved
src/components/search_bar/query/operators.ts Outdated Show resolved Hide resolved
src/components/search_bar/query/operators.ts Outdated Show resolved Hide resolved
src/components/search_bar/search_bar.tsx Outdated Show resolved Hide resolved
src/components/search_bar/search_box.tsx Outdated Show resolved Hide resolved
@pugnascotia
Copy link
Contributor Author

@chandlerprall back to you for re-review.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2909/

chandlerprall
chandlerprall suggested changes Mar 1, 2020
View reviewed changes
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Thanks! Found two more, after which I'll test these all of this against the existing usages in Kibana.

src/components/search_bar/search_filters.tsx Outdated Show resolved Hide resolved
src/components/search_bar/filters/field_value_selection_filter.tsx Outdated Show resolved Hide resolved
@pugnascotia
Copy link
Contributor Author

@chandlerprall back to you! 🏸

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2909/

chandlerprall
chandlerprall suggested changes Mar 4, 2020
View reviewed changes
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Did a Kibana pass using these types which found a couple things around onChange

src/components/search_bar/search_bar.tsx Outdated Show resolved Hide resolved
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2909/

chandlerprall
chandlerprall suggested changes Mar 5, 2020
View reviewed changes
Copy link
Contributor

@chandlerprall chandlerprall left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Couple more changes requested then this is great to go.

src/components/search_bar/query/query.ts Outdated Show resolved Hide resolved
chandlerprall
chandlerprall suggested changes Mar 5, 2020
View reviewed changes
src/components/search_bar/index.ts Outdated Show resolved Hide resolved
@pugnascotia
Copy link
Contributor Author

@chandlerprall back to you. I also defined a Syntax type.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2909/

chandlerprall
chandlerprall approved these changes Mar 6, 2020
View reviewed changes
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Good to go; Thanks @pugnascotia for taking this on!

@pugnascotia pugnascotia merged commit 63739a6 into elastic:master Mar 6, 2020
@pugnascotia pugnascotia deleted the convert-eui-search-bar-to-ts branch March 6, 2020 15:37
@thompsongl thompsongl mentioned this pull request Mar 12, 2020
Closed
@chandlerprall chandlerprall mentioned this pull request Mar 16, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chandlerprall chandlerprall chandlerprall approved these changes

@thompsongl thompsongl Awaiting requested review from thompsongl

Assignees
No one assigned
Labels
typescript
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pugnascotia @kibanamachine @chandlerprall