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

[App Search] Convert Search UI view to new page template + minor UI polish #102813

Merged
merged 6 commits into from
Jun 22, 2021

Conversation

cee-chen
Copy link
Member

Summary

Follow up to #102170 - converts more App Search pages to the new KibanaPageTemplate. I'm attempting to break up the AS layout conversion into smaller, easier to review chunks.

This PR handles the Search UI view, and additionally adds a new empty state for the page (discussed and approved by Davey last week). As always, follow along by commit (and turn off whitespace diffs)

Screencaps

New empty state:

Checklist

- On a totally new engine, all pages except this one* had an empty state, so per Davey's recommendations I whipped up a new empty state for this page

* Overview has a custom 'empty' state, analytics does not have an empty state
@cee-chen cee-chen added release_note:skip Skip the PR/issue when compiling release notes v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 21, 2021
@cee-chen cee-chen requested review from JasonStoltz and a team June 21, 2021 20:02
<p>
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.searchUI.empty.description', {
defaultMessage:
'A schema will be automatically created for you after you index some documents.',
Copy link
Member Author

Choose a reason for hiding this comment

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

This copy is the exact same as the empty states for relevance tuning & result settings, since all of these pages depend on/use engine schema

<AppSearchPageTemplate
pageChrome={getEngineBreadcrumbs([SEARCH_UI_TITLE])}
pageHeader={{ pageTitle: SEARCH_UI_TITLE }}
isEmptyState={Object.keys(engine.schema || {}).length === 0}
Copy link
Member Author

Choose a reason for hiding this comment

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

Heads up, I fully intend on refactoring this Object.keys check to an EngineLogic selector - but that's going to come in a follow-up polish PR once all the views land. Essentially I want to tackle the concept of empty engine polling more holistically and at the engine logic level, and ensure all pages update dynamically when an engine is empty and receives new documents/schema.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, I did check w/ Davey on this and he confirmed that this view should check for empty schema and not just for empty documents. So someone could index a document and create a schema but then delete the document, and this would no longer show the empty state (which matches how Relevance Tuning & Result Settings behave)

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
enterpriseSearch 1454 1455 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 2.1MB 2.1MB +909.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 22, 2021
…olish (elastic#102813)

* Convert Search UI view to use new page template

+ update tests TODO

* [UX polish] Add empty state to Search UI view

- On a totally new engine, all pages except this one* had an empty state, so per Davey's recommendations I whipped up a new empty state for this page

* Overview has a custom 'empty' state, analytics does not have an empty state

* Update router

* Fix bad merge conflict resolution

* [Polish] Copy feedback proposed by Davey

- see elastic@cbc3706
kibanamachine added a commit that referenced this pull request Jun 22, 2021
…olish (#102813) (#102952)

* Convert Search UI view to use new page template

+ update tests TODO

* [UX polish] Add empty state to Search UI view

- On a totally new engine, all pages except this one* had an empty state, so per Davey's recommendations I whipped up a new empty state for this page

* Overview has a custom 'empty' state, analytics does not have an empty state

* Update router

* Fix bad merge conflict resolution

* [Polish] Copy feedback proposed by Davey

- see cbc3706

Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants