Skip to content

Conversation

@Dosant
Copy link
Contributor

@Dosant Dosant commented Jun 15, 2023

Summary

Partially address #158835
Fix #159800

We're adding Drift to a couple of "high value" pages identified by the CSE team #158835. For now we don't add Drift globally because we have a bunch of concerns we need to resolve first #159691

Screenshot 2023-06-15 at 16 14 17 Screenshot 2023-06-15 at 16 14 32

To see how the chat bubble looks, you can use these URLs with my dummy test drift account:

@Dosant Dosant changed the title add drift to security getting start and observability overview [Drift] Add to security/get_started and observability/overview Jun 15, 2023
@Dosant Dosant added Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// Feature:Chat The integration of Live Chat (Drift) into Kibana. release_note:skip Skip the PR/issue when compiling release notes labels Jun 15, 2023
@Dosant Dosant marked this pull request as ready for review June 15, 2023 15:13
@Dosant Dosant requested review from a team as code owners June 15, 2023 15:13
@Dosant Dosant requested a review from a team June 15, 2023 15:13
@Dosant Dosant requested a review from a team as a code owner June 15, 2023 15:13
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

Core changes LGTM

@Dosant
Copy link
Contributor Author

Dosant commented Jun 19, 2023

@elasticmachine merge upstream

Copy link
Member

@machadoum machadoum left a comment

Choose a reason for hiding this comment

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

Explore chages LGTM!

Copy link
Member

@maryam-saeidi maryam-saeidi left a comment

Choose a reason for hiding this comment

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

AO changes LGTM

One question:
Were these warnings introduced in this PR?

  1. Warning: Unsupported style property z-index. Did you mean zIndex?

image

  1. Unsupported style property max-height. Did you mean maxHeight

image

@Dosant
Copy link
Contributor Author

Dosant commented Jun 19, 2023

@maryam-saeidi, yes, unfortunately, this is an issue with the Drift chat integration which needs some love. This is only a Dev warning and functionality wise it works as expected.

I'll create an issue to clean this up
#159904

@kibana-ci
Copy link

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #7 / Entity Analytics Dashboard With anomalies data renders table with pagination
  • [job] [logs] Security Solution Tests #7 / Prebuilt rules Actions with prebuilt rules Rules table "before each" hook for "Deletes and recovers more than one rule"

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
observability 517 518 +1

Async chunks

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

id before after diff
cloudChat 2.8KB 2.8KB -9.0B
observability 1024.0KB 1.0MB +175.0B
securitySolution 10.9MB 10.9MB +245.0B
total +411.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
observability 85.9KB 86.1KB +139.0B
securitySolution 51.3KB 51.4KB +139.0B
total +278.0B
Unknown metric groups

API count

id before after diff
observability 525 526 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 13 15 +2
securitySolution 411 415 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 14 16 +2
securitySolution 494 498 +4
total +6

History

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

Copy link
Member

@maryam-saeidi maryam-saeidi left a comment

Choose a reason for hiding this comment

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

@Dosant, I approve the PR since, as you mentioned, it is only a dev warning.

The challenge with these warnings is that they make it harder to notice valid issues in the developer console. Will you please make sure that the ticket will be picked up sooner? 🙂

@Dosant Dosant merged commit 3a27239 into elastic:main Jun 19, 2023
@kibanamachine kibanamachine added v8.9.0 backport:skip This PR does not require backporting labels Jun 19, 2023
@Dosant Dosant self-assigned this Jun 19, 2023
@sixstringcode
Copy link

@Dosant what is the easiest way to test that this is working for the Enterprise Search page? I can't create an 8.9 deployment trial.

@Dosant
Copy link
Contributor Author

Dosant commented Jul 25, 2023

@Dosant what is the easiest way to test that this is working for the Enterprise Search page? I can't create an 8.9 deployment trial.

@sixstringcode , This was added to enterprise search here: #124176
I don't know of a way of testing it in cloud without a trial

I myself only could checked this locally:
Screenshot 2023-07-24 at 12 34 37

@sixstringcode
Copy link

Thanks Anton, following the PRs cleared it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Chat The integration of Live Chat (Drift) into Kibana. release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// v8.9.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Drift] "Hide chat" button is missing

8 participants