From 79a001cdf538b12bdaf176d9b650ce8d25712a85 Mon Sep 17 00:00:00 2001 From: Dave Snider Date: Tue, 29 Sep 2020 15:01:29 -0700 Subject: [PATCH 01/10] Moving loading to logo, add a sligh 250ms pause --- .../loading_indicator.test.tsx.snap | 20 +- .../header/__snapshots__/header.test.tsx.snap | 231 ++++++++++-------- .../public/chrome/ui/header/elastic_mark.svg | 9 + src/core/public/chrome/ui/header/header.tsx | 2 +- .../public/chrome/ui/header/header_logo.scss | 12 + .../public/chrome/ui/header/header_logo.tsx | 25 +- .../chrome/ui/loading_indicator.test.tsx | 5 +- .../public/chrome/ui/loading_indicator.tsx | 38 ++- 8 files changed, 211 insertions(+), 131 deletions(-) create mode 100644 src/core/public/chrome/ui/header/elastic_mark.svg create mode 100644 src/core/public/chrome/ui/header/header_logo.scss diff --git a/src/core/public/chrome/ui/__snapshots__/loading_indicator.test.tsx.snap b/src/core/public/chrome/ui/__snapshots__/loading_indicator.test.tsx.snap index e6bf7e898d8c..10e6e9befe4f 100644 --- a/src/core/public/chrome/ui/__snapshots__/loading_indicator.test.tsx.snap +++ b/src/core/public/chrome/ui/__snapshots__/loading_indicator.test.tsx.snap @@ -1,19 +1,21 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`kbnLoadingIndicator is hidden by default 1`] = ` - `; exports[`kbnLoadingIndicator is visible when loadingCount is > 0 1`] = ` - `; diff --git a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap index 8937ecb4d9b4..19fc4ed870a2 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap @@ -1755,17 +1755,10 @@ exports[`Header renders 1`] = ` } } href="/" - navLinks$={ + loadingCount$={ BehaviorSubject { "_isScalar": false, - "_value": Array [ - Object { - "baseUrl": "", - "href": "", - "id": "kibana", - "title": "kibana", - }, - ], + "_value": 0, "closed": false, "hasError": false, "isStopped": false, @@ -1807,6 +1800,25 @@ exports[`Header renders 1`] = ` "syncErrorThrown": false, "syncErrorValue": null, }, + ], + "thrownError": null, + } + } + navLinks$={ + BehaviorSubject { + "_isScalar": false, + "_value": Array [ + Object { + "baseUrl": "", + "href": "", + "id": "kibana", + "title": "kibana", + }, + ], + "closed": false, + "hasError": false, + "isStopped": false, + "observers": Array [ Subscriber { "_parentOrParents": null, "_subscriptions": Array [ @@ -1844,21 +1856,6 @@ exports[`Header renders 1`] = ` "syncErrorThrown": false, "syncErrorValue": null, }, - ], - "thrownError": null, - } - } - navigateToApp={[MockFunction]} - />, - , ], }, @@ -2830,17 +2827,10 @@ exports[`Header renders 1`] = ` } } href="/" - navLinks$={ + loadingCount$={ BehaviorSubject { "_isScalar": false, - "_value": Array [ - Object { - "baseUrl": "", - "href": "", - "id": "kibana", - "title": "kibana", - }, - ], + "_value": 0, "closed": false, "hasError": false, "isStopped": false, @@ -2882,6 +2872,25 @@ exports[`Header renders 1`] = ` "syncErrorThrown": false, "syncErrorValue": null, }, + ], + "thrownError": null, + } + } + navLinks$={ + BehaviorSubject { + "_isScalar": false, + "_value": Array [ + Object { + "baseUrl": "", + "href": "", + "id": "kibana", + "title": "kibana", + }, + ], + "closed": false, + "hasError": false, + "isStopped": false, + "observers": Array [ Subscriber { "_parentOrParents": null, "_subscriptions": Array [ @@ -2919,66 +2928,6 @@ exports[`Header renders 1`] = ` "syncErrorThrown": false, "syncErrorValue": null, }, - ], - "thrownError": null, - } - } - navigateToApp={[MockFunction]} - > - - - - - - -
- - - + +
+ + + Elastic - - + +
diff --git a/src/core/public/chrome/ui/header/elastic_mark.svg b/src/core/public/chrome/ui/header/elastic_mark.svg new file mode 100644 index 000000000000..1b792ee1e9e3 --- /dev/null +++ b/src/core/public/chrome/ui/header/elastic_mark.svg @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/src/core/public/chrome/ui/header/header.tsx b/src/core/public/chrome/ui/header/header.tsx index 8a03c9c06845..6a639140f920 100644 --- a/src/core/public/chrome/ui/header/header.tsx +++ b/src/core/public/chrome/ui/header/header.tsx @@ -110,8 +110,8 @@ export function Header({ forceNavigation$={observables.forceAppSwitcherNavigation$} navLinks$={observables.navLinks$} navigateToApp={application.navigateToApp} + loadingCount$={observables.loadingCount$} />, - , ], borders: 'none', }, diff --git a/src/core/public/chrome/ui/header/header_logo.scss b/src/core/public/chrome/ui/header/header_logo.scss new file mode 100644 index 000000000000..2ea259737320 --- /dev/null +++ b/src/core/public/chrome/ui/header/header_logo.scss @@ -0,0 +1,12 @@ +.chrHeaderLogo { + height: $euiHeaderChildSize; + padding: 0 $euiSizeM; + display: inline-flex; + align-items: center; + vertical-align: middle; + white-space: nowrap; +} + +.chrHeaderLogo__mark { + margin-left: $euiSizeS; +} diff --git a/src/core/public/chrome/ui/header/header_logo.tsx b/src/core/public/chrome/ui/header/header_logo.tsx index 83e0c52ab3f3..110456e1b74a 100644 --- a/src/core/public/chrome/ui/header/header_logo.tsx +++ b/src/core/public/chrome/ui/header/header_logo.tsx @@ -17,13 +17,16 @@ * under the License. */ -import { EuiHeaderLogo } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import React from 'react'; import useObservable from 'react-use/lib/useObservable'; import { Observable } from 'rxjs'; import Url from 'url'; import { ChromeNavLink } from '../..'; +import elasticMark from './elastic_mark.svg'; +import { HttpStart } from '../../../http'; +import { LoadingIndicator } from '../loading_indicator'; +import './header_logo.scss'; function findClosestAnchor(element: HTMLElement): HTMLAnchorElement | void { let current = element; @@ -90,23 +93,31 @@ interface Props { navLinks$: Observable; forceNavigation$: Observable; navigateToApp: (appId: string) => void; + loadingCount$?: ReturnType; } -export function HeaderLogo({ href, navigateToApp, ...observables }: Props) { +export function HeaderLogo({ href, navigateToApp, loadingCount$, ...observables }: Props) { const forceNavigation = useObservable(observables.forceNavigation$, false); const navLinks = useObservable(observables.navLinks$, []); return ( - onClick(e, forceNavigation, navLinks, navigateToApp)} + className="euiHeaderSectionItem__button chrHeaderLogo" href={href} + data-test-subj="logo" aria-label={i18n.translate('core.ui.chrome.headerGlobalNav.goHomePageIconAriaLabel', { defaultMessage: 'Go to home page', })} > - Elastic - + + Elastic + ); } diff --git a/src/core/public/chrome/ui/loading_indicator.test.tsx b/src/core/public/chrome/ui/loading_indicator.test.tsx index ff56ca668ae0..2d45a3d07961 100644 --- a/src/core/public/chrome/ui/loading_indicator.test.tsx +++ b/src/core/public/chrome/ui/loading_indicator.test.tsx @@ -32,7 +32,10 @@ describe('kbnLoadingIndicator', () => { it('is visible when loadingCount is > 0', () => { const wrapper = shallow(); - expect(wrapper.prop('data-test-subj')).toBe('globalLoadingIndicator'); + // Pause the check beyond the 250ms delay that it has + setTimeout(() => { + expect(wrapper.prop('data-test-subj')).toBe('globalLoadingIndicator'); + }, 300); expect(wrapper).toMatchSnapshot(); }); }); diff --git a/src/core/public/chrome/ui/loading_indicator.tsx b/src/core/public/chrome/ui/loading_indicator.tsx index ca3e95f722ec..874fb3bc3e11 100644 --- a/src/core/public/chrome/ui/loading_indicator.tsx +++ b/src/core/public/chrome/ui/loading_indicator.tsx @@ -17,7 +17,7 @@ * under the License. */ -import { EuiLoadingSpinner, EuiProgress } from '@elastic/eui'; +import { EuiLoadingSpinner, EuiProgress, EuiIcon } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import React from 'react'; import classNames from 'classnames'; @@ -39,16 +39,26 @@ export class LoadingIndicator extends React.Component { - this.setState({ - visible: count > 0, - }); + if (this.increment > 1) { + clearTimeout(this.timer); + } + this.increment += this.increment; + this.timer = setTimeout(() => { + this.setState({ + visible: count > 0, + }); + }, 250); }); } componentWillUnmount() { if (this.loadingCountSubscription) { + clearTimeout(this.timer); this.loadingCountSubscription.unsubscribe(); this.loadingCountSubscription = undefined; } @@ -67,13 +77,27 @@ export class LoadingIndicator extends React.Component + ) : ( + + ); + + return !this.props.showAsBar ? ( + logo ) : ( Date: Tue, 29 Sep 2020 15:40:03 -0700 Subject: [PATCH 02/10] ts expect error for testing --- src/core/public/chrome/ui/header/header_logo.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/public/chrome/ui/header/header_logo.tsx b/src/core/public/chrome/ui/header/header_logo.tsx index 110456e1b74a..770b142df82c 100644 --- a/src/core/public/chrome/ui/header/header_logo.tsx +++ b/src/core/public/chrome/ui/header/header_logo.tsx @@ -23,6 +23,7 @@ import useObservable from 'react-use/lib/useObservable'; import { Observable } from 'rxjs'; import Url from 'url'; import { ChromeNavLink } from '../..'; +// @ts-expect-error TS doesn't like importing an SVG import elasticMark from './elastic_mark.svg'; import { HttpStart } from '../../../http'; import { LoadingIndicator } from '../loading_indicator'; From d87f1754de0f5ba3092a456c214a1c52d1be5c83 Mon Sep 17 00:00:00 2001 From: cchaos Date: Tue, 29 Sep 2020 18:53:41 -0400 Subject: [PATCH 03/10] Convert mark to a TS element - Removed inline fills for CSS implementation - Use the `euiHeaderLogo` class and remove custom styles --- .../header/__snapshots__/header.test.tsx.snap | 47 ++++++++++++++++--- .../public/chrome/ui/header/elastic_mark.svg | 9 ---- .../public/chrome/ui/header/elastic_mark.tsx | 42 +++++++++++++++++ .../public/chrome/ui/header/header_logo.scss | 10 +--- .../public/chrome/ui/header/header_logo.tsx | 15 ++---- 5 files changed, 88 insertions(+), 35 deletions(-) delete mode 100644 src/core/public/chrome/ui/header/elastic_mark.svg create mode 100644 src/core/public/chrome/ui/header/elastic_mark.tsx diff --git a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap index 19fc4ed870a2..6d1babc48050 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap @@ -2973,7 +2973,7 @@ exports[`Header renders 1`] = ` > - Elastic + > + + + Elastic + + + + + + + + + + diff --git a/src/core/public/chrome/ui/header/elastic_mark.svg b/src/core/public/chrome/ui/header/elastic_mark.svg deleted file mode 100644 index 1b792ee1e9e3..000000000000 --- a/src/core/public/chrome/ui/header/elastic_mark.svg +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - - - diff --git a/src/core/public/chrome/ui/header/elastic_mark.tsx b/src/core/public/chrome/ui/header/elastic_mark.tsx new file mode 100644 index 000000000000..f4d1d2f561f5 --- /dev/null +++ b/src/core/public/chrome/ui/header/elastic_mark.tsx @@ -0,0 +1,42 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React, { HTMLAttributes } from 'react'; +import { CommonProps } from '@elastic/eui'; + +export const ElasticMark = ({ ...props }: HTMLAttributes) => ( + + Elastic + + + + + + + + +); diff --git a/src/core/public/chrome/ui/header/header_logo.scss b/src/core/public/chrome/ui/header/header_logo.scss index 2ea259737320..f75fd9cfa246 100644 --- a/src/core/public/chrome/ui/header/header_logo.scss +++ b/src/core/public/chrome/ui/header/header_logo.scss @@ -1,12 +1,4 @@ -.chrHeaderLogo { - height: $euiHeaderChildSize; - padding: 0 $euiSizeM; - display: inline-flex; - align-items: center; - vertical-align: middle; - white-space: nowrap; -} - .chrHeaderLogo__mark { margin-left: $euiSizeS; + fill: $euiColorGhost; } diff --git a/src/core/public/chrome/ui/header/header_logo.tsx b/src/core/public/chrome/ui/header/header_logo.tsx index 770b142df82c..0311fd026131 100644 --- a/src/core/public/chrome/ui/header/header_logo.tsx +++ b/src/core/public/chrome/ui/header/header_logo.tsx @@ -17,17 +17,16 @@ * under the License. */ +import './header_logo.scss'; import { i18n } from '@kbn/i18n'; import React from 'react'; import useObservable from 'react-use/lib/useObservable'; import { Observable } from 'rxjs'; import Url from 'url'; import { ChromeNavLink } from '../..'; -// @ts-expect-error TS doesn't like importing an SVG -import elasticMark from './elastic_mark.svg'; +import { ElasticMark } from './elastic_mark'; import { HttpStart } from '../../../http'; import { LoadingIndicator } from '../loading_indicator'; -import './header_logo.scss'; function findClosestAnchor(element: HTMLElement): HTMLAnchorElement | void { let current = element; @@ -104,7 +103,7 @@ export function HeaderLogo({ href, navigateToApp, loadingCount$, ...observables return ( onClick(e, forceNavigation, navLinks, navigateToApp)} - className="euiHeaderSectionItem__button chrHeaderLogo" + className="euiHeaderLogo" href={href} data-test-subj="logo" aria-label={i18n.translate('core.ui.chrome.headerGlobalNav.goHomePageIconAriaLabel', { @@ -112,13 +111,7 @@ export function HeaderLogo({ href, navigateToApp, loadingCount$, ...observables })} > - Elastic + ); } From 744bbb3c7e0b29cbf5f2762de707ba74a0cb5188 Mon Sep 17 00:00:00 2001 From: Dave Snider Date: Tue, 29 Sep 2020 16:43:50 -0700 Subject: [PATCH 04/10] remove common props --- src/core/public/chrome/ui/header/elastic_mark.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/public/chrome/ui/header/elastic_mark.tsx b/src/core/public/chrome/ui/header/elastic_mark.tsx index f4d1d2f561f5..b7fe369e1181 100644 --- a/src/core/public/chrome/ui/header/elastic_mark.tsx +++ b/src/core/public/chrome/ui/header/elastic_mark.tsx @@ -18,7 +18,6 @@ */ import React, { HTMLAttributes } from 'react'; -import { CommonProps } from '@elastic/eui'; export const ElasticMark = ({ ...props }: HTMLAttributes) => ( Date: Tue, 29 Sep 2020 17:01:07 -0700 Subject: [PATCH 05/10] i18n --- src/core/public/chrome/ui/loading_indicator.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/public/chrome/ui/loading_indicator.tsx b/src/core/public/chrome/ui/loading_indicator.tsx index 874fb3bc3e11..25ec52e8dbb5 100644 --- a/src/core/public/chrome/ui/loading_indicator.tsx +++ b/src/core/public/chrome/ui/loading_indicator.tsx @@ -90,7 +90,7 @@ export class LoadingIndicator extends React.Component From 75fd6d8b7479ddaccffbc48b53da9ee28b460a67 Mon Sep 17 00:00:00 2001 From: Dave Snider Date: Wed, 30 Sep 2020 19:08:43 -0700 Subject: [PATCH 06/10] smaller svg --- src/core/public/chrome/ui/header/elastic_mark.tsx | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/core/public/chrome/ui/header/elastic_mark.tsx b/src/core/public/chrome/ui/header/elastic_mark.tsx index b7fe369e1181..1afb77180c89 100644 --- a/src/core/public/chrome/ui/header/elastic_mark.tsx +++ b/src/core/public/chrome/ui/header/elastic_mark.tsx @@ -23,19 +23,15 @@ export const ElasticMark = ({ ...props }: HTMLAttributes) => ( Elastic - - - - - - - + ); From c25bf1f33af914a09a07aee2f90617259a95692b Mon Sep 17 00:00:00 2001 From: Dave Snider Date: Wed, 30 Sep 2020 19:10:18 -0700 Subject: [PATCH 07/10] update tests --- .../header/__snapshots__/header.test.tsx.snap | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap index 6d1babc48050..5979cfd42dc2 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap @@ -3054,9 +3054,8 @@ exports[`Header renders 1`] = ` aria-hidden={true} aria-labelledby="elasticMark" className="chrHeaderLogo__mark" - fill="currentColor" + fill="none" height="19" - viewBox="0 0 64 19" width="64" xmlns="http://www.w3.org/2000/svg" > @@ -3066,25 +3065,8 @@ exports[`Header renders 1`] = ` Elastic - - - - - - From 0339431c468c3ccc1d85377b8848cec045062da2 Mon Sep 17 00:00:00 2001 From: Dave Snider Date: Tue, 6 Oct 2020 17:42:58 -0700 Subject: [PATCH 08/10] feedback --- src/core/public/chrome/ui/header/header_logo.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/public/chrome/ui/header/header_logo.tsx b/src/core/public/chrome/ui/header/header_logo.tsx index 0311fd026131..df961ebb0983 100644 --- a/src/core/public/chrome/ui/header/header_logo.tsx +++ b/src/core/public/chrome/ui/header/header_logo.tsx @@ -107,7 +107,7 @@ export function HeaderLogo({ href, navigateToApp, loadingCount$, ...observables href={href} data-test-subj="logo" aria-label={i18n.translate('core.ui.chrome.headerGlobalNav.goHomePageIconAriaLabel', { - defaultMessage: 'Go to home page', + defaultMessage: 'Elastic home', })} > From 26b95c53667b198e572b493d9f0c97c2893df3e6 Mon Sep 17 00:00:00 2001 From: Dave Snider Date: Tue, 6 Oct 2020 18:39:47 -0700 Subject: [PATCH 09/10] tests --- .../public/chrome/ui/header/__snapshots__/header.test.tsx.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap index 7d5dd8c32dd1..f1580d051fc8 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap @@ -2963,7 +2963,7 @@ exports[`Header renders 1`] = ` navigateToApp={[MockFunction]} > Date: Wed, 7 Oct 2020 08:35:16 -0700 Subject: [PATCH 10/10] feedback --- .../chrome/ui/header/__snapshots__/header.test.tsx.snap | 1 - src/core/public/chrome/ui/header/elastic_mark.tsx | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap index f1580d051fc8..5e563c406109 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap @@ -3057,7 +3057,6 @@ exports[`Header renders 1`] = ` diff --git a/src/core/public/chrome/ui/header/elastic_mark.tsx b/src/core/public/chrome/ui/header/elastic_mark.tsx index 1afb77180c89..e4456e9b751f 100644 --- a/src/core/public/chrome/ui/header/elastic_mark.tsx +++ b/src/core/public/chrome/ui/header/elastic_mark.tsx @@ -29,9 +29,6 @@ export const ElasticMark = ({ ...props }: HTMLAttributes) => ( {...props} > Elastic - + );