[Telemetry] Update notice message by afharo · Pull Request #158669 · elastic/kibana

afharo · 2023-05-30T16:18:23Z

Summary

This PR revisits the message shown in the opt-in/out status notices/disclaimers/banners in Kibana.

It uses, when possible, the same component so we can hold the same message to be reused in the Welcome page and the notice banner.
It merges the notice banners (we had 2 to mostly show the same info)
It changes the text used in the Advanced Settings section to align with a similar narrative, as reviewed by @cdanderson98.
It adds translated terms to the Advanced Settings search (related to [Telemetry] Telemetry Advanced settings section should use the regular UI registration API #56498)
It removes usages of Usage data in favor of Usage collection (related to @gchaps' comment [DOCS] Telemetry settings: improve phrasing #158396 (comment))

How does it look?

Welcome page

When enabled (default):

When disabled:

Notice banner

When enabled (default):

When disabled:

Advanced settings

When enabled (default):

When disabled:

Before the first review

Welcome page

When enabled (default):

When disabled:

Notice banner

When enabled (default):

When disabled:

Advanced settings

When enabled (default):

When disabled:

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

github-actions · 2023-05-30T16:18:36Z

Documentation preview:

✨ Changed pages

…fix'

elasticmachine · 2023-05-30T21:42:59Z

Pinging @elastic/kibana-core (Team:Core)

elasticmachine · 2023-05-30T21:43:08Z

Pinging @elastic/kibana-docs (Team:Docs)

src/plugins/telemetry/public/components/opt_in_message.tsx

src/plugins/telemetry_management_section/public/components/telemetry_management_section.tsx

...anagement_section/public/components/__snapshots__/telemetry_management_section.test.tsx.snap

src/plugins/telemetry_management_section/public/components/telemetry_management_section.tsx

docs/settings/telemetry-settings.asciidoc

afharo · 2023-06-01T07:26:50Z

@gchaps, thank you for your comments. I'll address them today and update the screenshots with the new content.

…e-notice-message

jloleysens

Great work @afharo ! Did not test locally, but the test coverage looks great.

Thanks for adding the screenshots. WDYT about making the status of usage collection bold or italicised?

packages/core/saved-objects/core-saved-objects-api-server/src/saved_objects_repository.ts

jloleysens · 2023-06-01T09:51:07Z

src/plugins/telemetry/public/services/telemetry_service.test.ts

+          optIn: null,
+        },
+      });
+      expect(telemetryService.config.userCanChangeSettings).toBe(true);


unrelated to this PR and non-blocker: is there a reason config is exposed on the telemetryService? Seems that the useCanChangeSettings is the same value just synced from config?

yeah... there is/was a bit of a nightmare config vs. satinized config (especially for the optIn value, where we differentiate the actual value and the expected behaviour, and both are used in different places)...

We def need to revisit this. I bet we can remove many of the public APIs in this service 😇.

jloleysens · 2023-06-01T09:53:18Z

src/plugins/telemetry/public/services/telemetry_notifications/telemetry_notifications.test.ts

+    telemetryService.getUserShouldSeeOptInNotice = getUserShouldSeeOptInNotice;
    const telemetryNotifications = mockTelemetryNotifications({ telemetryService });
-    expect(telemetryNotifications.shouldShowOptedInNoticeBanner()).toBe(false);
+    telemetryNotifications['optInStatusNoticeBannerId'] = 'bruce-banner';


src/plugins/telemetry/public/services/telemetry_notifications/telemetry_notifications.test.ts

…saved_objects_repository.ts Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>

…telemetry_notifications.test.ts Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>

afharo · 2023-06-01T10:27:53Z

Thanks for adding the screenshots. WDYT about making the status of usage collection bold or italicised?

@jloleysens, I went with italics because it serves the purpose of catching the eye when reading the message.

Bold felt like bringing attention from any place on the page (interrupting other onboarding flows).

Please, let me know if you prefer it bold.

jloleysens · 2023-06-01T10:33:01Z

@afharo thanks for sharing that. It's a question that invites deep philosophical reflection and product thinking 😄

To help answer the question I would ask: how much attention do we want to draw to this? For me I'd want users to be maximally aware and so lean toward bold as this is a good way to draw attention to softer grey text that is already de-emphasised.

OTOH, we might also choose to leave it as is (regular weight, not italicised). But that is the product part of the question. Bolding that status might lead to more users opting-out.

afharo · 2023-06-01T10:38:17Z

@jloleysens I pinged @cdanderson98 (from legal) to let us know his thoughts. Also pinging @arisonl (PM) and @gchaps (docs) to complete the different POVs 😇

afharo · 2023-06-01T10:40:10Z

Probably, @elastic/kibana-design also has something to say here. WRT the discussion of highlighting the current status of telemetry (enabled/disabled) in bold, or italics, or none.

kibana-ci · 2023-06-01T11:44:23Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 489981c

Failed CI Steps

FTR Configs #17

Test Failures

[job] [logs] FTR Configs #17 / spaces api without security _disable_legacy_url_aliases "before all" hook for "should return 204 for alias 'default:sharedtype:space_1_only'"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`telemetry`	32	30	-2

Async chunks

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

id	before	after	diff
`telemetry`	4.9KB	4.0KB	-941.0B
`telemetryManagementSection`	10.3KB	10.6KB	+322.0B
total			-619.0B

Page load bundle

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

id	before	after	diff
`telemetry`	19.7KB	18.9KB	-795.0B

Unknown metric groups

async chunk count

id	before	after	diff
`telemetry`	3	2	-1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`securitySolution`	415	419	+4
`telemetry`	1	3	+2
total			+8

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`securitySolution`	499	503	+4
`telemetry`	6	8	+2
total			+8

History

💚 Build #131123 succeeded bef8ea8
💔 Build #131059 failed 54bd993

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

cc @afharo

afharo · 2023-06-01T15:52:55Z

I don't want to delay the important update that PR brings in terms of the text we show to our users, so I created the follow-up issue #158841 to discuss whether we prefer bold or italics 😇

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com> (cherry picked from commit 312ba3a)

kibanamachine · 2023-06-01T15:59:18Z

💚 All backports created successfully

Status	Branch	Result
✅	8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

# Backport This will backport the following commits from `main` to `8.8`: - [[Telemetry] Update notice message (#158669)](#158669)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Alejandro Fernández Haro <alejandro.haro@elastic.co>

[Telemetry] Update notice message

b66cbc8

afharo added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels May 30, 2023

afharo requested a review from cdanderson98 May 30, 2023 16:18

afharo self-assigned this May 30, 2023

kibanamachine and others added 3 commits May 30, 2023 16:25

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

be258d3

…fix'

Remove dot and add space

54bd993

Fix tests

bef8ea8

afharo marked this pull request as ready for review May 30, 2023 21:42

afharo requested a review from a team as a code owner May 30, 2023 21:42

afharo added the Team:Docs label May 30, 2023

afharo requested a review from a team May 30, 2023 21:43

gchaps reviewed May 31, 2023

View reviewed changes

afharo added 2 commits June 1, 2023 11:17

Merge branch 'main' of github.com:elastic/kibana into telemetry/updat…

4e096fa

…e-notice-message

Address PR feedback

9e95c0f

afharo requested a review from gchaps June 1, 2023 09:49

jloleysens approved these changes Jun 1, 2023

View reviewed changes

This comment was marked as duplicate.

Sign in to view

afharo and others added 3 commits June 1, 2023 12:08

Update packages/core/saved-objects/core-saved-objects-api-server/src/…

e2a2a3a

…saved_objects_repository.ts Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>

Update src/plugins/telemetry/public/services/telemetry_notifications/…

7d81477

…telemetry_notifications.test.ts Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>

Status in italics

489981c

afharo mentioned this pull request Jun 1, 2023

[Telemetry] Highlighting the current status: **bold** or _italics_? #158841

Closed

afharo added backport:prev-minor and removed backport:skip This PR does not require backporting labels Jun 1, 2023

afharo merged commit 312ba3a into elastic:main Jun 1, 2023

afharo deleted the telemetry/update-notice-message branch June 1, 2023 15:53

kibanamachine added the v8.9.0 label Jun 1, 2023

kibanamachine mentioned this pull request Jun 1, 2023

[8.8] [Telemetry] Update notice message (#158669) #158842

Merged

kibanamachine added the v8.8.1 label Jun 1, 2023

This was referenced Jun 7, 2023

Advanced settings UI does not support localized search terms #88345

Closed

[Telemetry] Telemetry Advanced settings section should use the regular UI registration API #56498

Open

rseldner mentioned this pull request Jun 16, 2023

Telemetry notice banner dismissal does not persist in Kibana 8.8.1 #159870

Closed

Conversation

afharo commented May 30, 2023 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How does it look?

Welcome page

Notice banner

Advanced settings

Welcome page

Notice banner

Advanced settings

Checklist

For maintainers

Uh oh!

github-actions bot commented May 30, 2023

Uh oh!

elasticmachine commented May 30, 2023

Uh oh!

elasticmachine commented May 30, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

afharo commented Jun 1, 2023

Uh oh!

jloleysens left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jloleysens Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

afharo Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

jloleysens Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as duplicate.

Uh oh!

afharo commented Jun 1, 2023

Uh oh!

jloleysens commented Jun 1, 2023

Uh oh!

afharo commented Jun 1, 2023

Uh oh!

afharo commented Jun 1, 2023

Uh oh!

kibana-ci commented Jun 1, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

Page load bundle

async chunk count

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

afharo commented Jun 1, 2023

Uh oh!

kibanamachine commented Jun 1, 2023

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

afharo commented May 30, 2023 •

edited by kibanamachine

Loading