-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[App Search] Updates to results on the documents view #86181
Conversation
if (!shouldLinkToDetailPage) | ||
return <article className="appSearchResult__content">{children}</article>; | ||
return ( | ||
<ReactRouterHelper to={getDocumentDetailRoute(resultMeta.engine, resultMeta.id)}> |
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.
This was super helpful, thanks @constancecchen!
84f5789
to
3166601
Compare
...ns/enterprise_search/public/applications/app_search/components/result/result_field_value.tsx
Show resolved
Hide resolved
3166601
to
a17a127
Compare
@constancecchen FYI, an issue I noticed while creating this PR, which I'll fix separately. When you're on the documents view for a meta engine, and you click a result, it links you to a document detail page within another engine context. This currently fails because it attempts to fetch data using the engine that is currently stored in EngineLogic, which doesn't update (at least not soon enough). Just thought I'd mention it. |
a17a127
to
5d67e96
Compare
x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts
Show resolved
Hide resolved
...lic/applications/app_search/components/documents/search_experience/build_search_ui_config.ts
Show resolved
Hide resolved
...pplications/app_search/components/documents/search_experience/build_search_ui_config.test.ts
Show resolved
Hide resolved
...lic/applications/app_search/components/documents/search_experience/build_search_ui_config.ts
Show resolved
Hide resolved
...s/enterprise_search/public/applications/app_search/components/result/result_field_value.scss
Show resolved
Hide resolved
x-pack/plugins/enterprise_search/public/applications/app_search/components/library/library.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result.tsx
Outdated
Show resolved
Hide resolved
shouldLinkToDetailPage={true} | ||
schemaForTypeHighlights={schemaForTypeHighlights} |
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.
Love these super clear prop names ✨
x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result.tsx
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.
This all looks super great! 🎉 I think my only semi blocking comment is #86181 (comment) and figuring out if that test is needed or if we should drop the || {}
, everything else is optional comments.
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.
🎉 Awesome work!
sortField: 'id', | ||
}, | ||
searchQuery: { | ||
result_fields: Object.keys(schema || {}).reduce( |
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.
Ah it looks like the || {}
got added back in somehow by a newer commit
result_fields: Object.keys(schema || {}).reduce( | |
result_fields: Object.keys(schema).reduce( |
💛 Build succeeded, but was flaky
Test FailuresChrome UI Functional Tests.test/functional/apps/discover/_discover·ts.discover app discover test query should modify the time range when the histogram is brushedStandard Out
Stack Trace
Metrics [docs]Module Count
Async chunks
Distributable file count
History
To update your PR or re-run it, just comment with: |
* master: (48 commits) Fix request with disabled aggregation (elastic#85696) [Security Solution][Detections][Threshold Rules] Threshold Rule Bug Fixes (elastic#84918) Removed a possibility to define two different names for Alert types on API and UI level. (elastic#86236) Bump Node.js from version 14.15.2 to 14.15.3 (elastic#86593) [index patterns] Fleep app - Keep saved object field list until field caps provides fields (elastic#85370) [Security Solutions] fix timeline tabs + layout (elastic#86581) Upgrade to hapi version 20 (elastic#85406) App Services: Remove remaining uiActions, expressions, data, embeddable circular dependencies. (elastic#82791) Rename chartLibrary setting to legacyChartsLibrary (elastic#86529) [CI] TeamCity updates (elastic#85843) [Maps] Use Json for mvt-tests (elastic#86492) [Rollup Jobs] Added autofocus to cron editor (elastic#86324) [Monitoring][Alerting] CCR read exceptions alert (elastic#85908) [CI] Bump memory for main CI workers (elastic#86541) Explicitly set Elasticsearch heap size during CI and local development (elastic#86513) [App Search] Updates to results on the documents view (elastic#86181) [Discover] Change default sort handling (elastic#85561) [App Search] Convert DocumentCreationModal to DocumentCreationFlyout (elastic#86508) [App Search] Sample Engines should have access to the Crawler (elastic#86502) Fixed duplication of create new modal (elastic#86489) ...
Summary
This PR makes 3 updates to the results on the documents view.
Checklist
Delete any items that are not applicable to this PR.
For maintainers