[App Search] Engine overview layout stub #83504

cee-chen · 2020-11-16T23:20:57Z

Summary

This PR stubs out the Engine Overview layout. It is still currently missing the following components, which will be added next:

CodeExample
DocumentCreationButtons & DocumentCreationModal
Analytics chart (high lift)
API Logs Table (this will likely be done significantly later / we migrate the API table view, although I also might give this a shot over the quieter December holidays, we'll see)

Screencaps

Populated Engine Metrics

Empty Engine

Engine data unavailable/still indexing

(not sure I've ever seen this personally)

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

cee-chen · 2020-11-16T23:23:30Z

...k/plugins/enterprise_search/public/applications/app_search/components/analytics/constants.ts

+export const TOTAL_CLICKS = i18n.translate(
+  'xpack.enterpriseSearch.appSearch.engine.analytics.totalClicks',
+  { defaultMessage: 'Total Clicks' }
+);


These constants are also used in the Analytics view so I figured I'd do myself a favor and start a constants file early.

cee-chen · 2020-11-16T23:23:46Z

...ck/plugins/enterprise_search/public/applications/app_search/components/api_logs/constants.ts

+export const RECENT_API_EVENTS = i18n.translate(
+  'xpack.enterpriseSearch.appSearch.engine.apiLogs.recent',
+  { defaultMessage: 'Recent API Events' }
+);


See above, this title is also used in the API Logs view

cee-chen · 2020-11-16T23:25:18Z

...erprise_search/public/applications/app_search/components/engine_overview/components/index.ts

+export { UnavailablePrompt } from './unavailable_prompt';
+export { TotalStats } from './total_stats';
+export { TotalCharts } from './total_charts';
+export { RecentLogs } from './recent_logs';


I went back and forth a little on whether I should split out the metrics view into sub-components, and I'm very happy I did in the end. The code feels much easier to test and reason about in smaller bite-sized files.

I left the empty engine view as one piece/component but definitely open to splitting that up as well if folks think that will help readability.

cee-chen · 2020-11-16T23:26:24Z

...search/public/applications/app_search/components/engine_overview/components/total_charts.tsx

+            TODO: Analytics chart
+            {/* <EngineAnalytics
+              data={[queriesPerDay]}
+              startDate={new Date(startDate)}
+              endDate={new Date(endDate)}
+            /> */}


To be clear I'm not 100% on if this will translate perfectly to EuiChart as I haven't spiked that out much, I'm just leaving this in as reference from our old code

cee-chen · 2020-11-16T23:27:25Z

.../public/applications/app_search/components/engine_overview/components/unavailable_prompt.tsx

+
+export const UnavailablePrompt: React.FC = () => (
+  <EuiEmptyPrompt
+    iconType="clock"


I've never actually seen this prompt before, so this was my best guesstimate on migrating a STUI icon to EUI

...erprise_search/public/applications/app_search/components/engine_overview/engine_overview.tsx

...rch/public/applications/app_search/components/engine_overview/engine_overview_empty.test.tsx

- Minus document creation button & API code example

- They're repeated/reused by the DocumentCreationPopover component

cee-chen · 2020-11-17T03:29:46Z

...e_search/public/applications/app_search/components/engine_overview/engine_overview_empty.tsx

+        </EuiPageContentHeader>
+        <EuiPageContentBody>
+          <EuiText color="subdued">
+            <p>{DOCUMENT_CREATION_DESCRIPTION}</p>


I just discovered this (and the API indexing title/description) is reused word for word in the Index Documents modal, so I pulled it out to a constants file:

It makes the page a tad hard to "read" without looking at the actual compiled app, but what can you do 🤷

JasonStoltz

I had some comments and questions, but none of it's blocking. Feel free to merge as is, or address as you see fit.

.../enterprise_search/public/applications/app_search/components/document_creation/constants.tsx

..._search/public/applications/app_search/components/engine_overview/components/recent_logs.tsx

JasonStoltz · 2020-11-18T14:37:54Z

..._search/public/applications/app_search/components/engine_overview/components/recent_logs.tsx

+  const engineRoute = getEngineRoute(engineName);
+
+  return (
+    <EuiPageContent>


Semantically, are pages meant to have multiple EuiPageContent in EUI? Or is there some other tag available for page sections?

Semantically I don't see a reason why not as they're not using <main> components under the hood (which is what we're using for EuiPageBody) - they're just plain <div>s which carry no extra meaning to web crawlers or screen readers.

..._search/public/applications/app_search/components/engine_overview/components/recent_logs.tsx

JasonStoltz · 2020-11-18T15:25:52Z

.../public/applications/app_search/components/engine_overview/components/unavailable_prompt.tsx

+    title={
+      <h2>
+        {i18n.translate('xpack.enterpriseSearch.appSearch.engine.overview.unavailableTitle', {
+          defaultMessage: 'Dashboard metrics are currently unavailable',


Suggested change

defaultMessage: 'Dashboard metrics are currently unavailable',

defaultMessage: 'Dashboard metrics are currently unavailable.',

👍 For a lot of these casing comments, I was copying these over from ent-search as-is, but I think it makes a lot of sense to update them as we migrate. Will do so going forward!

I went back and looked at some of EuiEmptyPrompt examples and I think the headings are actually not supposed to have punctuation: https://elastic.github.io/eui/#/display/empty-prompt and

So going to leave this one as-is 🤷

Makes sense 👍

...erprise_search/public/applications/app_search/components/engine_overview/engine_overview.tsx

JasonStoltz · 2020-11-18T15:47:36Z

...e_search/public/applications/app_search/components/engine_overview/engine_overview_empty.tsx

+          <EuiTitle size="l">
+            <h1>
+              {i18n.translate('xpack.enterpriseSearch.appSearch.engine.overview.empty.heading', {
+                defaultMessage: 'Engine Setup',


Per the Kibana writing guidelines, I think stuff like this should not be Title Cased. I'm actually unsure of this one.

Suggested change

defaultMessage: 'Engine Setup',

defaultMessage: 'Engine setup',

@chriscressman Do you mind weighing in on this here? Should we be updating various headings and buttons to use sentence case over title case as we migrate to Kibana?

Yes, we should consistently use sentence case in all our UIs. This is a company-wide style rule. The Kibana migration is a great opportunity to align. Thanks for checking!

Also @JasonStoltz - should Engine overview also be sentence cased vs title cased in that case? 🤔

Yes, I think ideally we would change all heading and button text within these screens to use sentence case.

@gchaps is the expert on Kibana (and Elastic) UI text. Can you confirm Gail?

Thanks Chris! I think my confusion is what's considered a "Feature Title" vs not. "Overview" is technically the title of the page and is capitalized in the navigation, does that make it a feature that should remain capitalized?

Sorry for opening this can of worms 😬

...e_search/public/applications/app_search/components/engine_overview/engine_overview_empty.tsx

...rch/public/applications/app_search/components/engine_overview/engine_overview_empty.test.tsx

chriscressman

Set up the "empty" Engine

I'd change this to:

Setting up the "empty" engine

since the text in that section is explaining the process rather than providing a step-by-step procedure. The second form is also consistent with the next heading on the page.

Other than the suggestion above, and the suggestion to use title casing, this looks good to me.

cee-chen · 2020-11-18T19:17:49Z

Re: copy - I'll definitely make an effort to grab any low-hanging fruit going forward, but App Search definitely has a lot of scenarios where it's unclear what's a Feature Title/proper noun vs over-enthusiastic title casing. We also have a lot of code to shift and a tight release deadline without much wiggle-room, and we can't necessarily afford to wait for extra reviews to merge PRs. My proposal going forward is:

Schedule a one-off session w/ Chris and/or Gail and go over the current standalone App Search app and have them point out examples of correct vs incorrect casing so we have a consistent set of rules to draw from going forward

or

Postpone copy changes for after 7.13 FF, at which point we have between then and 7.14 FF to do a copy sweep of the app

Let me know your thoughts and what you have time for, Chris and Gail! ❤️

chriscressman · 2020-11-18T21:17:23Z

@constancecchen

Postpone copy changes for after 7.13 FF, at which point we have between then and 7.14 FF to do a copy sweep of the app

I think this might be better for everyone. Personally, I prefer to work in an async flow. I tend to structure my day, so I ignore notifications during large chunks. I don't want copy changes to block progress.

But for reference, Elastic feature names should not be capitalized:

Features of the Elastic Stack (anomaly detection, index lifecycle management, etc.) are not capitalized.

-- from the Elastic naming guide

So feature names like engine and analytics should be lowercase, except when the first word in a sentence. I corrected my earlier comment, where I wrote Engine instead of engine.

cee-chen · 2020-11-18T21:50:57Z

That is super helpful, thank you Chris!! I think I'm starting to get it now 🎉

I've updated copy in this PR, will be doing another quick pass of the rest of our current app, and have created a follow-up icebox ticket to remind myself to come back and you amazing copy folks in 7.13 / April 2021 when the full app is ready. Until then!! 🙇‍♀️

gchaps · 2020-11-18T22:00:02Z

@chriscressman, @constancecchen Agree that it will be good to do a copy review in a single swoop. That way, we can make sure that the text flows from one page to the next.

kibanamachine · 2020-11-18T23:52:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: cc01d6b

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`enterpriseSearch`	468	482	+14

Async chunks

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

id	before	after	diff
`enterpriseSearch`	692.0KB	710.3KB	+18.3KB

History

💔 Build #88762 failed 51f20a89a80bf3953b26829bef8560bd32982cbe
💚 Build #88711 succeeded 9bc3780
💔 Build #88692 failed 7c6c8c8d3937c6c081603976781e6a5d7d07c2fa
💚 Build #88216 succeeded 2fca086
💚 Build #88211 succeeded 2be4c35

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

* Set up Overview file * Finish Overview page logic, stub out empty/metric views * Stub in basic empty engine overview - Minus document creation button & API code example * Stub out EngineOverviewMetrics and unavailable empty prompt * Stub out EngineOverMetrics components (stats, charts, logs) * [Refactor] Pull out some document creation i18n strings to constants - They're repeated/reused by the DocumentCreationPopover component * PR feedback: Drop the regex * PR feedback: RecentLogs -> RecentApiLogs * PR feedback: Copy * PR feedback: Copy, sentence-casing

This reverts commit 71f972d.

spalger · 2020-11-19T01:37:15Z

Sorry, this started to fail as soon as it was merged into master. I've reverted it for now to leave master broken for as little time as possible. Please resubmit with master merged, thanks! I also suggest closing the backport and backporting the resubmit-PR instead.

* master: (60 commits) Forward any registry cache-control header for files (elastic#83680) Revert "[Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578)" [Security Solution][Detections] Fix adding an action to detection rules (elastic#83722) Make expectSnapshot available in all functional test runs (elastic#82932) Skip failing cypress test Increase bulk request timeout during esArchiver load (elastic#83657) [data.search] Server-side background session service (elastic#81099) [maps] convert VectorStyleEditor to TS (elastic#83582) Revert "[App Search] Engine overview layout stub (elastic#83504)" Adding documentation for global action configuration options (elastic#83557) [Metrics UI] Optimizations for Snapshot and Inventory Metadata (elastic#83596) chore(NA): update lmdb store to v0.8.15 (elastic#83726) [App Search] Engine overview layout stub (elastic#83504) [Workplace Search] Update SourceIcon to match latest changes in ent-search (elastic#83714) [Enterprise Search] Rename React Router helpers (elastic#83718) [Maps] Add 'crossed' & 'exited' events to tracking alert (elastic#82463) Updating code-owners to use new core/app-services team names (elastic#83731) Add Managed label to data streams and a view switch for the table (elastic#83049) [Maps] Add query bar inputs to geo threshold alerts tracked points & boundaries (elastic#80871) fix(NA): search examples kibana version declaration (elastic#83182) ...

…ode-details * 'master' of github.com:elastic/kibana: fixed pagination in connectors list (elastic#83638) Forward any registry cache-control header for files (elastic#83680) Revert "[Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578)" [Security Solution][Detections] Fix adding an action to detection rules (elastic#83722) Make expectSnapshot available in all functional test runs (elastic#82932) Skip failing cypress test Increase bulk request timeout during esArchiver load (elastic#83657) [data.search] Server-side background session service (elastic#81099) [maps] convert VectorStyleEditor to TS (elastic#83582) Revert "[App Search] Engine overview layout stub (elastic#83504)" Adding documentation for global action configuration options (elastic#83557) [Metrics UI] Optimizations for Snapshot and Inventory Metadata (elastic#83596) chore(NA): update lmdb store to v0.8.15 (elastic#83726) [App Search] Engine overview layout stub (elastic#83504) [Workplace Search] Update SourceIcon to match latest changes in ent-search (elastic#83714) [Enterprise Search] Rename React Router helpers (elastic#83718)

* Set up Overview file * Finish Overview page logic, stub out empty/metric views * Stub in basic empty engine overview - Minus document creation button & API code example * Stub out EngineOverviewMetrics and unavailable empty prompt * Stub out EngineOverMetrics components (stats, charts, logs) * [Refactor] Pull out some document creation i18n strings to constants - They're repeated/reused by the DocumentCreationPopover component * PR feedback: Drop the regex * PR feedback: RecentLogs -> RecentApiLogs * PR feedback: Copy * PR feedback: Copy, sentence-casing

This reverts commit 71f972d.

cee-chen added Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Nov 16, 2020

cee-chen requested review from JasonStoltz and a team November 16, 2020 23:20

cee-chen added 2 commits November 16, 2020 15:22

Set up Overview file

fa9d12c

Finish Overview page logic, stub out empty/metric views

7ac0e3b

cee-chen force-pushed the engine-overview branch from 6de0b10 to dc9afe3 Compare November 16, 2020 23:22

cee-chen commented Nov 16, 2020

View reviewed changes

cee-chen force-pushed the engine-overview branch 2 times, most recently from 2644fa9 to fdf1a4d Compare November 17, 2020 01:21

cee-chen added 3 commits November 16, 2020 17:32

Stub in basic empty engine overview

0d71c73

- Minus document creation button & API code example

Stub out EngineOverviewMetrics and unavailable empty prompt

1097787

Stub out EngineOverMetrics components (stats, charts, logs)

2be4c35

cee-chen force-pushed the engine-overview branch from fdf1a4d to 2be4c35 Compare November 17, 2020 01:32

[Refactor] Pull out some document creation i18n strings to constants

2fca086

- They're repeated/reused by the DocumentCreationPopover component

cee-chen commented Nov 17, 2020

View reviewed changes

JasonStoltz approved these changes Nov 18, 2020

View reviewed changes

cee-chen added 3 commits November 18, 2020 10:12

Merge branch 'master' into engine-overview

4cd44f9

PR feedback: Drop the regex

5e945c3

PR feedback: RecentLogs -> RecentApiLogs

4d4747c

chriscressman reviewed Nov 18, 2020

View reviewed changes

PR feedback: Copy

9bc3780

cee-chen force-pushed the engine-overview branch from 7c6c8c8 to 9bc3780 Compare November 18, 2020 19:51

cee-chen mentioned this pull request Nov 18, 2020

[Enterprise Search] Rename React Router helpers #83718

Merged

3 tasks

cee-chen force-pushed the engine-overview branch from dbf42c0 to 51f20a8 Compare November 18, 2020 21:49

PR feedback: Copy, sentence-casing

cc01d6b

cee-chen force-pushed the engine-overview branch from 51f20a8 to cc01d6b Compare November 18, 2020 22:11

chriscressman approved these changes Nov 18, 2020

View reviewed changes

cee-chen merged commit 71f972d into elastic:master Nov 18, 2020

cee-chen deleted the engine-overview branch November 18, 2020 23:53

cee-chen mentioned this pull request Nov 18, 2020

[7.x] [App Search] Engine overview layout stub (#83504) #83747

Closed

spalger added a commit that referenced this pull request Nov 19, 2020

Revert "[App Search] Engine overview layout stub (#83504)"

92acf45

This reverts commit 71f972d.

spalger added the reverted label Nov 19, 2020

cee-chen mentioned this pull request Nov 19, 2020

[Enterprise Search] Engine overview layout stub #83756

Merged

chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Nov 19, 2020

Revert "[App Search] Engine overview layout stub (elastic#83504)"

c366228

This reverts commit 71f972d.

cee-chen mentioned this pull request Feb 2, 2021

[App Search] Migrate Create Engine view #89816

Merged

7 tasks

JasonStoltz mentioned this pull request Feb 16, 2021

[App Search] First cut of the Relevance Tuning UI #90621

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[App Search] Engine overview layout stub #83504

[App Search] Engine overview layout stub #83504

cee-chen commented Nov 16, 2020

cee-chen Nov 16, 2020

cee-chen Nov 16, 2020

cee-chen Nov 16, 2020

cee-chen Nov 16, 2020

cee-chen Nov 16, 2020

cee-chen Nov 17, 2020 •

edited

Loading

JasonStoltz left a comment

JasonStoltz Nov 18, 2020

cee-chen Nov 18, 2020

JasonStoltz Nov 18, 2020

cee-chen Nov 18, 2020

cee-chen Nov 18, 2020 •

edited

Loading

JasonStoltz Nov 18, 2020

JasonStoltz Nov 18, 2020

cee-chen Nov 18, 2020

chriscressman Nov 18, 2020

cee-chen Nov 18, 2020

chriscressman Nov 18, 2020

cee-chen Nov 18, 2020

JasonStoltz Nov 18, 2020

chriscressman left a comment •

edited

Loading

cee-chen commented Nov 18, 2020

chriscressman commented Nov 18, 2020

cee-chen commented Nov 18, 2020

gchaps commented Nov 18, 2020

kibanamachine commented Nov 18, 2020

spalger commented Nov 19, 2020

	defaultMessage: 'Dashboard metrics are currently unavailable',
	defaultMessage: 'Dashboard metrics are currently unavailable.',

	defaultMessage: 'Engine Setup',
	defaultMessage: 'Engine setup',

[App Search] Engine overview layout stub #83504

[App Search] Engine overview layout stub #83504

Conversation

cee-chen commented Nov 16, 2020

Summary

Screencaps

Populated Engine Metrics

Empty Engine

Engine data unavailable/still indexing

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cee-chen Nov 17, 2020 • edited Loading

Choose a reason for hiding this comment

JasonStoltz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cee-chen Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriscressman left a comment • edited Loading

Choose a reason for hiding this comment

cee-chen commented Nov 18, 2020

chriscressman commented Nov 18, 2020

cee-chen commented Nov 18, 2020

gchaps commented Nov 18, 2020

kibanamachine commented Nov 18, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

spalger commented Nov 19, 2020

cee-chen Nov 17, 2020 •

edited

Loading

cee-chen Nov 18, 2020 •

edited

Loading

chriscressman left a comment •

edited

Loading