-
Notifications
You must be signed in to change notification settings - Fork 8k
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
[APM] Context-aware query examples for the query bar #38915
Conversation
@@ -23,6 +23,7 @@ type BreadcrumbFunction = (props: LocationMatch) => string; | |||
|
|||
export interface BreadcrumbRoute extends RouteProps { | |||
breadcrumb: string | BreadcrumbFunction | null; | |||
name: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if this is the right place, but seems overkill to add another type on top of this. maybe we should rename this to APMRoute
?
x-pack/plugins/apm/public/components/app/Main/UpdateBreadcrumbs.tsx
Outdated
Show resolved
Hide resolved
183b61f
to
0fc02c4
Compare
💔 Build Failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good @dgieselaar just one minor copy edit for the general query placeholder example.
💔 Build Failed |
x-pack/plugins/apm/public/components/app/Main/UpdateBreadcrumbs.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
f5e19af
to
6be6023
Compare
💔 Build Failed |
6be6023
to
367ceed
Compare
💚 Build Succeeded |
367ceed
to
6a761c3
Compare
💔 Build Failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I have a few comments, that can be taken or left as is.
x-pack/legacy/plugins/apm/public/components/app/Main/route_config/route_names.tsx
Outdated
Show resolved
Hide resolved
We now adjust the query example based on whether the user is viewing transactions, errors or metrics.
6a761c3
to
bb18aa8
Compare
bb18aa8
to
6efd857
Compare
💔 Build Failed |
retest |
💚 Build Succeeded |
* [APM] Context-aware query examples for the query bar We now adjust the query example based on whether the user is viewing transactions, errors or metrics. * Change query example for transactions * Address review feedback * Fix ts issues in unit tests * Use enum for route names, clarify queryExample w/ comment
Closes #37224.
We now adjust the query example based on whether the user is viewing transactions, errors or metrics.
Before:
![image](https://user-images.githubusercontent.com/352732/59455689-03293380-8e15-11e9-90dc-e50e30b76f8c.png)
After:
![image](https://user-images.githubusercontent.com/352732/59455640-e8ef5580-8e14-11e9-99be-4b50a930756a.png)
![image](https://user-images.githubusercontent.com/352732/59455631-e1c84780-8e14-11e9-9dae-221324a15408.png)
Definition of Done for APM UI
Flag and create issues of things that are left outPM approval for bigger, user-facing features