Add tech preview label for search applications #155649

ioanatia · 2023-04-24T16:21:14Z

Adds a tech preview popover and acknowledgement for search applications.
The popover is visible on mouse hover.

On platinum+/trial:

other licenses (e.g. basic):

ioanatia · 2023-04-24T16:24:28Z

...earch/public/applications/enterprise_search_content/components/engines/engines_list.test.tsx

@@ -120,34 +120,6 @@ describe('EnginesList', () => {

 describe('CreateEngineButton', () => {
  describe('disabled={true}', () => {
-    it('renders a disabled button that shows a popover when focused', () => {


I repurposed the popover for the platinum license acknowledgement based on the design.
The initial platinum license popover was only visible when not on an platinum+/trial license (and hidden on trial/platinum+)
The Create Search Application would also be disabled when not on platinum+/trial.
Because the popover is now visible regardless of the license level and whether the create button is disabled, I removed part of the tests.

kderusso

LGTM

joemcelroy · 2023-04-24T20:20:07Z

...ch/public/applications/enterprise_search_content/components/engines/create_engine_flyout.tsx

+        <EuiFlexGroup direction="column">
+          <EuiFlexItem grow>
+            <EuiCallOut title="Technical Preview feature" color="warning" iconType="beaker">
+              <p>


title and description are missing translations. Guide on how to do + some examples below of how i18n util is used.

joemcelroy · 2023-04-24T20:23:02Z

...ise_search/public/applications/enterprise_search_content/components/engines/engines_list.tsx

@@ -81,21 +80,18 @@ export const CreateEngineButton: React.FC<CreateEngineButtonProps> = ({ disabled
    >
      <EuiPopoverTitle>
        <FormattedMessage
-          id="xpack.enterpriseSearch.content.searchApplications.createEngineDisabledPopover.title"
-          defaultMessage="Platinum only feature"
+          id="xpack.enterpriseSearch.content.searchApplications.createEngineTechnicalPreviewPopover.title"


run node scripts/i18n_check --fix for updating removed i18n messages if they have been translated already.

julianrosado · 2023-04-24T20:46:57Z

@ioanatia Thanks for getting this up so fast! Some feedback

Is it possible to add the badge on top of the title like the original design? This is proposed to be used in other features like that, so if it's not technically feasible then I'd like to flag! I don't think having a tooltip on the button is strong enough to let people know it's in tech preview.

1b. If the badge is not possible for some reason, then I'd want to copy over the callout from the flyout and have it sit below the platinum license gate callout.

Small copy changes for "Search Applications" consistency courtesy of @leemthompo and UX simplicity courtesy of me!:

For the header:

Search Applications help make your Elasticsearch data searchable for end users, with out-of-the-box relevance, analytics and personalization tools.To learn more about how search applications work in Enterprise Search explore our Search Applications documentation.

For the empty state title:

Create your first search application

(and remove the copy below that title, just title + button)

For all button instances (to shorten those bad boys up):

"Create Search Application" -> "Create"

In the flyout subhead, reduce text down to:

Explore our Search Applications documentation to learn more.

In the flyout step 2:

Name your search application

In the flyout step 2 placeholder text:

Search application name

Small nice to have nitpick: Can the popover with technical preview title have the beaker icon to make it in line with the badge and callout box versions?

julianrosado · 2023-04-24T23:58:33Z

Per Nick and Serena feedback, if we can do the above then we can lose the paragraph below the popover too!

...ch/public/applications/enterprise_search_content/components/engines/create_engine_flyout.tsx

...ise_search/public/applications/enterprise_search_content/components/engines/engines_list.tsx

julianrosado · 2023-04-25T13:21:45Z

...ise_search/public/applications/enterprise_search_content/components/engines/engines_list.tsx

+            <FormattedMessage
+              id="xpack.enterpriseSearch.content.searchApplications.createEngineTechnicalPreviewPopover.title"
+              defaultMessage="Technical Preview"
+            />


Not sure why (in your screenshot) there is so much space between the icon and the title text but wanted to flag

looks like the space is controlled by gutterSize TIL
should look a bit more decent now:

That works for me Ioana!

…-ref HEAD~1..HEAD --fix'

sloanelybutsurely

just one note about keyboard navigability

sloanelybutsurely · 2023-04-25T15:50:09Z

...ise_search/public/applications/enterprise_search_content/components/engines/engines_list.tsx

      closePopover={() => setShowPopover(false)}
      button={
        <div
          data-test-subj="create-engine-button-hover-target"
          onMouseEnter={() => setShowPopover(true)}
-          onFocus={() => setShowPopover(true)}
+          onMouseLeave={() => setShowPopover(false)}


we may still want a focus event for users navigating with a keyboard

Yes, we should definitely have that. I am also concerned about other potential accessibility issues here. Does the popover announce its presence when opened? Does tabIndex 0 make sense in the context of this page? Does onFocus get triggered when someone tabs to the button, or only when it tabs to the div?

I'll get these fixed in a follow up PR. thanks for the feedback

kibana-ci · 2023-04-25T16:47:30Z

💚 Build Succeeded

Buildkite Build
Commit: 9e3b5a8

Metrics [docs]

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.3MB	2.3MB	-4.0KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	17	19	+2
`securitySolution`	396	399	+3
total			+5

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	18	20	+2
`securitySolution`	476	479	+3
total			+5

History

💛 Build #122870 was flaky 12e875f
💚 Build #122574 succeeded 4e6c110

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

* main: (1294 commits) [SecuritySolution] Refactor security packages (elastic#155365) [Discover] Show "Temporary" badge for ad-hoc data views in Alerts flyout (elastic#155717) [RAM] Conditional actions feedback on pr review (elastic#155804) [Files] Adds bulk delete method (elastic#155628) [Lens] Use proper way to generate absolute short URL (elastic#155512) [Guided onboarding] Use Kibana features to grant access (elastic#155065) [Index Management] Fix duped mock (elastic#155844) [Lens] Enhance visualization modifier popup with layer palette (elastic#155280) Fix flaky combobox tests on role management screen (elastic#155711) [Infrastructure UI] Create InventoryViewsService and InventoryViewsClient (elastic#155126) [Fleet] always create agent upload write indices (elastic#155729) [Fleet] [Cloud Security Posture] Add CloudFormation agent install method (elastic#155045) Add tech preview label for search applications (elastic#155649) [ML] AIOps: Stabilize flaky functional tests. (elastic#155710) [ES UI Shared] Migrate JsonEditor to monaco (elastic#155610) [Security Solution] Fixes security_solution storybooks always rendering in a flyout (elastic#155814) [Synthetics] Make error popover disappear `onMouseLeave` of metric item card (elastic#155800) Remove Exploratory View components from Observability (elastic#155629) [Discover] Remove redundant "Filter was added" toast (elastic#155645) [RAM][Security Solution][Alerts] Support the ability to trigger a rule action per alert generated (elastic#153611) (elastic#155384) ...

julianrosado · 2023-04-26T14:16:09Z

...ch/public/applications/enterprise_search_content/components/engines/create_engine_flyout.tsx

+            <EuiCallOut title="Technical Preview feature" color="warning" iconType="beaker">
+              <FormattedMessage
+                id="xpack.enterpriseSearch.content.engines.createEngine.technicalPreviewCallOut.title"
+                defaultMessage="This functionality is in technical preview and may be changed or removed completely in a future release. Elastic will take a best effort approach to fix any issues, but features in technical preview are not subject to the support SLA of official GA features."


Can we quickly change this to say:

This functionality may be changed or removed completely in a future release.

To make it less wordy

Just realized this was in the wrong place, need this for the popover, not the callout. Callout is fine as is.

Add tech preview label for search applications

4e6c110

ioanatia added release_note:skip Skip the PR/issue when compiling release notes Team:EnterpriseSearch v8.8.0 labels Apr 24, 2023

ioanatia commented Apr 24, 2023

View reviewed changes

ioanatia marked this pull request as ready for review April 24, 2023 19:10

ioanatia requested review from a team, TattdCodeMonkey and saarikabhasi April 24, 2023 19:10

kderusso approved these changes Apr 24, 2023

View reviewed changes

joemcelroy reviewed Apr 24, 2023

View reviewed changes

ioanatia added 4 commits April 25, 2023 11:43

Copy changes

8719ae5

Add beaker icon in popover title

7814259

Use FormattedMessage

589f29a

Merge branch 'main' into search_apps_tech_preview

12e875f

joemcelroy approved these changes Apr 25, 2023

View reviewed changes

julianrosado reviewed Apr 25, 2023

View reviewed changes

...ch/public/applications/enterprise_search_content/components/engines/create_engine_flyout.tsx Outdated Show resolved Hide resolved

julianrosado reviewed Apr 25, 2023

View reviewed changes

...ch/public/applications/enterprise_search_content/components/engines/create_engine_flyout.tsx Outdated Show resolved Hide resolved

julianrosado reviewed Apr 25, 2023

View reviewed changes

...ise_search/public/applications/enterprise_search_content/components/engines/engines_list.tsx Outdated Show resolved Hide resolved

julianrosado reviewed Apr 25, 2023

View reviewed changes

Address review comments

5e9d4bc

ioanatia requested a review from julianrosado April 25, 2023 15:25

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

9e3b5a8

…-ref HEAD~1..HEAD --fix'

sloanelybutsurely approved these changes Apr 25, 2023

View reviewed changes

ioanatia merged commit 783c893 into elastic:main Apr 26, 2023

kibanamachine added the backport:skip This commit does not require backporting label Apr 26, 2023

julianrosado reviewed Apr 26, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tech preview label for search applications #155649

Add tech preview label for search applications #155649

ioanatia commented Apr 24, 2023

ioanatia Apr 24, 2023

kderusso left a comment

joemcelroy Apr 24, 2023

joemcelroy Apr 24, 2023

julianrosado commented Apr 24, 2023

julianrosado commented Apr 24, 2023

julianrosado Apr 25, 2023

ioanatia Apr 25, 2023

julianrosado Apr 26, 2023

sloanelybutsurely left a comment

sloanelybutsurely Apr 25, 2023

sphilipse Apr 25, 2023

ioanatia Apr 26, 2023

kibana-ci commented Apr 25, 2023

ESLint disabled line counts

Total ESLint disabled count

julianrosado Apr 26, 2023

julianrosado Apr 27, 2023

Add tech preview label for search applications #155649

Add tech preview label for search applications #155649

Conversation

ioanatia commented Apr 24, 2023

Choose a reason for hiding this comment

kderusso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julianrosado commented Apr 24, 2023

julianrosado commented Apr 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sloanelybutsurely 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

kibana-ci commented Apr 25, 2023

💚 Build Succeeded

Metrics [docs]

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

Choose a reason for hiding this comment

Choose a reason for hiding this comment