Add doc titles to ES UI apps #71045

cjcenizal · 2020-07-08T05:03:03Z

Applies to CCR, ILM, Index Management, Ingest Node Pipelines, License Management, Remote Clusters, Rollup Jobs, Watcher, and Upgrade Assistant. Also, this PR clears the doc title when leaving Dev Tools.

elasticmachine · 2020-07-08T05:03:06Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

…License Management, Remote Clusters, Rollup Jobs, and Upgrade Assistant. Clear doc title when leaving Dev Tools.

jloleysens · 2020-07-14T12:58:37Z

@elasticmachine merge upstream

jloleysens

@cjcenizal Great work and thanks for fixing this! I see my original issue was missing "Watcher" from this list. Would you mind also updating that mount logic there?

jloleysens · 2020-07-14T13:24:42Z

src/plugins/dev_tools/public/application.tsx

@@ -203,6 +203,7 @@ export function renderApp(
  });

  return () => {
+    chrome.docTitle.reset();


I think we could add this to the management plugin (in OSS) too, so that when the management area renders/mounts, before any plugins have rendered, we always reset the doc title. Instead of in the clean up function we do this on init.

I see what you're saying, and I think I'd like to encourage each app to clean up after itself instead. I think it's likely that we'll move apps out of Management and when that happens it will be reassuring to know that this behavior is encapsulated within each plugin and won't be broken by the move.

jloleysens · 2020-07-14T13:25:59Z

x-pack/plugins/cross_cluster_replication/public/plugin.ts

@@ -60,6 +63,12 @@ export class CrossClusterReplicationPlugin implements Plugin {
          history,
          getUrlForApp,
        });
+
+        return () => {
+          // Change tab label back to Kibana.


FWIW I think this comment should be updated to "...back to Elastic" because that is the actual value. I think we could actually remove this comment entirely as I think the function name and object name are descriptive enough :)

cjcenizal · 2020-07-17T22:07:45Z

Thanks for the review @jloleysens! I've removed the comments and I decided not to act on your other suggestion. Could you please take another look?

jloleysens · 2020-07-20T10:16:32Z

@elasticmachine merge upstream

jloleysens · 2020-07-20T10:17:36Z

@cjcenizal Thanks for addressing the point regarding the comments. Happy with your decision regarding management plugin.

However I still think we should include fixing watcher's page title here.

jloleysens · 2020-07-20T10:17:58Z

Per my review comment :)

- Refactor boot file to follow index-oriented pattern of other plugins.

cjcenizal · 2020-07-20T20:52:54Z

@jloleysens Thanks for reminding me about Watcher! Done. Can you take another look?

jloleysens

Thanks for fixing watcher too!

All looks good to me now, great work @cjcenizal !

jloleysens · 2020-07-21T08:51:21Z

@elasticmachine merge upstream

kibanamachine · 2020-07-21T15:49:30Z

💚 Build Succeeded

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

Build metrics

async chunks size

id	value	diff	baseline
devTools	6.5KB	+24.0B	6.4KB
indexManagement	1.5MB	+219.0B	1.5MB
watcher	599.4KB	+62.0B	599.3KB
total	-	+305.0B	-

page load bundle size

id	value	diff	baseline
crossClusterReplication	192.2KB	+223.0B	192.0KB
indexLifecycleManagement	238.2KB	+177.0B	238.1KB
ingestPipelines	32.4KB	+1.8KB	30.7KB
licenseManagement	27.0KB	+197.0B	26.8KB
remoteClusters	35.1KB	+263.0B	34.8KB
rollup	224.7KB	+1.7KB	223.0KB
upgradeAssistant	51.9KB	+1.8KB	50.2KB
watcher	36.3KB	+240.0B	36.1KB
total	-	+6.3KB	-

History

💚 Build #63218 succeeded fa7455a
💚 Build #63128 succeeded 96e9ce6
💚 Build #62908 succeeded ae8ab3b
💚 Build #62777 succeeded 2a51575
💔 Build #62764 failed f7d5c4a

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

* Add doc titles to CCR, ILM, Index Management, Ingest Node Pipelines, License Management, Remote Clusters, Rollup Jobs, Watcher, and Upgrade Assistant. Clear doc title when leaving Dev Tools. * Refactor Watcher boot file to follow index-oriented pattern of other plugins.

* master: [Security Solution][Timeline] Add Empty view to the Timelines page (elastic#72576) [Security Solution][Resolver] Show process detail panel when clicking a process node (elastic#72563) Move manifest packageConfig mocks into security_solution plugin (elastic#72527) [QA][Code Coverage] Fixup Team Assignment (elastic#72467) [docs] remove references to tile map visualization in supported aggregations (elastic#72493) [ci][apm-ui] fix argument name for disabling pr comments (elastic#72633) Only check that the event ids are the same in arrays (elastic#72624) Add doc titles to ES UI apps (elastic#71045) Add Upgrade Assistant API integration test to ensure the reindex operation saved object can handle immense error messages (elastic#72347) [APM] Disable flaky rum e2e’s (elastic#72614) Applying tiny fix from 72532 to main branch (elastic#72533) [APM] Update script with new roles/users (elastic#72599) [Security Solution] Add margin (elastic#72542) Migrated fixed_scroll karma tests to jest (elastic#72258) [ML] Handling data recognizer saved object errors (elastic#72447) [Monitoring] Fix the messaging around needing TLS enabled (elastic#72310) [Task Manager] Batches the update operations in Task Manager (elastic#71470)

…feature-privileges * alerting/consumer-based-rbac: [Security Solution][Timeline] Add Empty view to the Timelines page (elastic#72576) [Security Solution][Resolver] Show process detail panel when clicking a process node (elastic#72563) renamed variable to make it clear the SO client is unsecured Move manifest packageConfig mocks into security_solution plugin (elastic#72527) [QA][Code Coverage] Fixup Team Assignment (elastic#72467) [docs] remove references to tile map visualization in supported aggregations (elastic#72493) [ci][apm-ui] fix argument name for disabling pr comments (elastic#72633) Only check that the event ids are the same in arrays (elastic#72624) includes hidden params type in SO client Add doc titles to ES UI apps (elastic#71045) Add Upgrade Assistant API integration test to ensure the reindex operation saved object can handle immense error messages (elastic#72347) [APM] Disable flaky rum e2e’s (elastic#72614) Applying tiny fix from 72532 to main branch (elastic#72533) [APM] Update script with new roles/users (elastic#72599) [Security Solution] Add margin (elastic#72542) Migrated fixed_scroll karma tests to jest (elastic#72258) [ML] Handling data recognizer saved object errors (elastic#72447) [Monitoring] Fix the messaging around needing TLS enabled (elastic#72310) [Task Manager] Batches the update operations in Task Manager (elastic#71470)

cjcenizal requested a review from jloleysens July 8, 2020 05:03

cjcenizal requested a review from a team as a code owner July 8, 2020 05:03

Add doc titles to CCR, ILM, Index Management, Ingest Node Pipelines, …

661db66

…License Management, Remote Clusters, Rollup Jobs, and Upgrade Assistant. Clear doc title when leaving Dev Tools.

cjcenizal force-pushed the bug/es-ui-apps-doc-titles branch from b190726 to 661db66 Compare July 8, 2020 15:07

cjcenizal added v7.10.0 and removed v7.9.0 labels Jul 13, 2020

jloleysens requested changes Jul 14, 2020

View reviewed changes

Remove obvious comments.

f7d5c4a

cjcenizal force-pushed the bug/es-ui-apps-doc-titles branch from b3d8093 to f7d5c4a Compare July 17, 2020 21:27

Merge branch 'master' into bug/es-ui-apps-doc-titles

2a51575

Merge branch 'master' into bug/es-ui-apps-doc-titles

ae8ab3b

Render Watcher title in tab and clear it on unmount.

96e9ce6

- Refactor boot file to follow index-oriented pattern of other plugins.

jloleysens approved these changes Jul 21, 2020

View reviewed changes

Merge branch 'master' into bug/es-ui-apps-doc-titles

fa7455a

cjcenizal merged commit a540caf into elastic:master Jul 21, 2020

cjcenizal deleted the bug/es-ui-apps-doc-titles branch July 21, 2020 14:04

sulemanof mentioned this pull request Jul 29, 2020

Reset chrome fields while switching an app #73064

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add doc titles to ES UI apps #71045

Add doc titles to ES UI apps #71045

cjcenizal commented Jul 8, 2020 •

edited

Loading

elasticmachine commented Jul 8, 2020

jloleysens commented Jul 14, 2020

jloleysens left a comment

jloleysens Jul 14, 2020

cjcenizal Jul 17, 2020 •

edited

Loading

jloleysens Jul 14, 2020

cjcenizal commented Jul 17, 2020 •

edited

Loading

jloleysens commented Jul 20, 2020

jloleysens commented Jul 20, 2020

jloleysens commented Jul 20, 2020

cjcenizal commented Jul 20, 2020

jloleysens left a comment

jloleysens commented Jul 21, 2020

kibanamachine commented Jul 21, 2020

Add doc titles to ES UI apps #71045

Add doc titles to ES UI apps #71045

Conversation

cjcenizal commented Jul 8, 2020 • edited Loading

elasticmachine commented Jul 8, 2020

jloleysens commented Jul 14, 2020

jloleysens left a comment

Choose a reason for hiding this comment

jloleysens Jul 14, 2020

Choose a reason for hiding this comment

cjcenizal Jul 17, 2020 • edited Loading

Choose a reason for hiding this comment

jloleysens Jul 14, 2020

Choose a reason for hiding this comment

cjcenizal commented Jul 17, 2020 • edited Loading

jloleysens commented Jul 20, 2020

jloleysens commented Jul 20, 2020

jloleysens commented Jul 20, 2020

cjcenizal commented Jul 20, 2020

jloleysens left a comment

Choose a reason for hiding this comment

jloleysens commented Jul 21, 2020

kibanamachine commented Jul 21, 2020

💚 Build Succeeded

Build metrics

async chunks size

page load bundle size

History

cjcenizal commented Jul 8, 2020 •

edited

Loading

cjcenizal Jul 17, 2020 •

edited

Loading

cjcenizal commented Jul 17, 2020 •

edited

Loading