From 32c572ae7eaad8355728465a3efd4148e8d76b36 Mon Sep 17 00:00:00 2001 From: Josh Dover Date: Sun, 19 Apr 2020 10:40:55 -0600 Subject: [PATCH] PR comments --- .../src/optimizer/get_bundles.ts | 2 +- src/core/public/core_system.test.ts | 2 +- src/core/public/entry_point.ts | 1 + src/core/public/index.scss | 49 ++++++++++++++- .../__snapshots__/legacy_service.test.ts.snap | 26 -------- src/core/public/legacy/legacy_service.test.ts | 55 ++++++++++++++++- src/legacy/ui/public/_index.scss | 1 - src/legacy/ui/public/chrome/_index.scss | 3 - src/legacy/ui/public/chrome/_variables.scss | 4 -- .../ui/public/chrome/directives/_index.scss | 1 - .../public/chrome/directives/_kbn_chrome.scss | 46 -------------- .../ui/ui_render/bootstrap/template.js.hbs | 37 ++---------- src/legacy/ui/ui_render/ui_render_mixin.js | 60 +++++++++++++------ src/optimize/bundles_route/bundles_route.js | 5 +- .../public/angular/templates/_graph.scss | 2 - 15 files changed, 152 insertions(+), 142 deletions(-) delete mode 100644 src/core/public/legacy/__snapshots__/legacy_service.test.ts.snap delete mode 100644 src/legacy/ui/public/chrome/_index.scss delete mode 100644 src/legacy/ui/public/chrome/_variables.scss delete mode 100644 src/legacy/ui/public/chrome/directives/_index.scss delete mode 100644 src/legacy/ui/public/chrome/directives/_kbn_chrome.scss diff --git a/packages/kbn-optimizer/src/optimizer/get_bundles.ts b/packages/kbn-optimizer/src/optimizer/get_bundles.ts index 110891565c27a5..b20ee6dcfdd538 100644 --- a/packages/kbn-optimizer/src/optimizer/get_bundles.ts +++ b/packages/kbn-optimizer/src/optimizer/get_bundles.ts @@ -30,7 +30,7 @@ export function getBundles(plugins: KibanaPlatformPlugin[], repoRoot: string) { entry: './public/entry_point', sourceRoot: repoRoot, contextDir: Path.resolve(repoRoot, 'src/core'), - outputDir: Path.resolve(repoRoot, 'src/core/public/target/public'), + outputDir: Path.resolve(repoRoot, 'src/core/target/public'), }); const pluginBundles = plugins diff --git a/src/core/public/core_system.test.ts b/src/core/public/core_system.test.ts index 559dadfc3dba4b..a42719417a2b17 100644 --- a/src/core/public/core_system.test.ts +++ b/src/core/public/core_system.test.ts @@ -103,7 +103,7 @@ describe('constructor', () => { }); }); - it('passes requireLegacyFiles, requireNewPlatformShimModule, requireLegacyBootstrapModule and a dom element to LegacyPlatformService', () => { + it('passes required params to LegacyPlatformService', () => { const requireLegacyFiles = { requireLegacyFiles: true }; const requireLegacyBootstrapModule = { requireLegacyBootstrapModule: true }; const requireNewPlatformShimModule = { requireNewPlatformShimModule: true }; diff --git a/src/core/public/entry_point.ts b/src/core/public/entry_point.ts index 847d5dbcb413b1..c40fc5247d28c4 100644 --- a/src/core/public/entry_point.ts +++ b/src/core/public/entry_point.ts @@ -22,6 +22,7 @@ * that lives in the Kibana Platform. */ +import './index.scss'; import { i18n } from '@kbn/i18n'; import { CoreSystem } from './core_system'; diff --git a/src/core/public/index.scss b/src/core/public/index.scss index 86f2efdff77020..104819632572ee 100644 --- a/src/core/public/index.scss +++ b/src/core/public/index.scss @@ -7,5 +7,52 @@ // Mixins provide generic code expansion through helpers @import '@elastic/eui/src/global_styling/mixins/index'; +$kbnGlobalNavClosedWidth: 53px; +$kbnGlobalNavOpenWidth: 180px; +$kbnGlobalNavLogoHeight: 70px; +$kbnGlobalNavAppIconHeight: $euiSizeXXL + $euiSizeXS; + +#kibana-body { + overflow-x: hidden; + min-height: 100%; +} + +.app-wrapper { + display: flex; + flex-flow: column nowrap; + position: absolute; + left: $kbnGlobalNavClosedWidth; + top: 0; + right: 0; + bottom: 0; + z-index: 5; + margin: 0 auto; + + /** + * 1. Dirty, but we need to override the .kbnGlobalNav-isOpen state + * when we're looking at the log-in screen. + */ + &.hidden-chrome { + left: 0 !important; /* 1 */ + } + + .navbar-right { + margin-right: 0; + } +} + +.app-wrapper-panel { + display: flex; + flex-grow: 1; + flex-shrink: 0; + flex-basis: auto; + flex-direction: column; + + > * { + flex-shrink: 0; + } +} + + @import './chrome/index'; -@import './overlays/index'; +@import './overlays/index'; \ No newline at end of file diff --git a/src/core/public/legacy/__snapshots__/legacy_service.test.ts.snap b/src/core/public/legacy/__snapshots__/legacy_service.test.ts.snap deleted file mode 100644 index 304d748f88020c..00000000000000 --- a/src/core/public/legacy/__snapshots__/legacy_service.test.ts.snap +++ /dev/null @@ -1,26 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`#start() load order loads ui/modules before ui/chrome, and both before legacy files 1`] = ` -Array [ - "ui/chrome", - "legacy files", -] -`; - -exports[`#stop() destroys the angular scope and empties the targetDomElement if angular is bootstrapped to targetDomElement 1`] = ` -
-`; - -exports[`#stop() does nothing if angular was not bootstrapped to targetDomElement 1`] = ` -
- - -

- this should not be removed -

- - -
-`; diff --git a/src/core/public/legacy/legacy_service.test.ts b/src/core/public/legacy/legacy_service.test.ts index 73d615042ef041..fa29320aab4e67 100644 --- a/src/core/public/legacy/legacy_service.test.ts +++ b/src/core/public/legacy/legacy_service.test.ts @@ -124,6 +124,21 @@ describe('#setup()', () => { expect(mockUiNewPlatformSetup).toHaveBeenCalledTimes(1); expect(mockUiNewPlatformSetup).toHaveBeenCalledWith(expect.any(Object), {}); }); + + it('throws error if requireNewPlatformShimModule is undefined', () => { + const legacyPlatform = new LegacyPlatformService({ + ...defaultParams, + requireNewPlatformShimModule: undefined, + }); + + expect(() => { + legacyPlatform.setup(defaultSetupDeps); + }).toThrowErrorMatchingInlineSnapshot( + `"requireNewPlatformShimModule must be specified when rendering a legacy application"` + ); + + expect(mockUiNewPlatformSetup).not.toHaveBeenCalled(); + }); }); }); @@ -157,6 +172,21 @@ describe('#start()', () => { expect(mockUiNewPlatformStart).toHaveBeenCalledWith(expect.any(Object), {}); }); + it('throws error if requireNewPlatformShimeModule is undefined', () => { + const legacyPlatform = new LegacyPlatformService({ + ...defaultParams, + requireNewPlatformShimModule: undefined, + }); + + expect(() => { + legacyPlatform.start(defaultStartDeps); + }).toThrowErrorMatchingInlineSnapshot( + `"requireNewPlatformShimModule must be specified when rendering a legacy application"` + ); + + expect(mockUiNewPlatformStart).not.toHaveBeenCalled(); + }); + it('resolves getStartServices with core and plugin APIs', async () => { const legacyPlatform = new LegacyPlatformService({ ...defaultParams, @@ -194,7 +224,12 @@ describe('#start()', () => { legacyPlatform.setup(defaultSetupDeps); legacyPlatform.start(defaultStartDeps); - expect(mockLoadOrder).toMatchSnapshot(); + expect(mockLoadOrder).toMatchInlineSnapshot(` + Array [ + "ui/chrome", + "legacy files", + ] + `); }); }); }); @@ -211,7 +246,17 @@ describe('#stop()', () => { }); legacyPlatform.stop(); - expect(targetDomElement).toMatchSnapshot(); + expect(targetDomElement).toMatchInlineSnapshot(` +
+ + +

+ this should not be removed +

+ + +
+ `); }); it('destroys the angular scope and empties the targetDomElement if angular is bootstrapped to targetDomElement', async () => { @@ -240,7 +285,11 @@ describe('#stop()', () => { legacyPlatform.start({ ...defaultStartDeps, targetDomElement }); legacyPlatform.stop(); - expect(targetDomElement).toMatchSnapshot(); + expect(targetDomElement).toMatchInlineSnapshot(` +
+ `); expect(scopeDestroySpy).toHaveBeenCalledTimes(1); }); }); diff --git a/src/legacy/ui/public/_index.scss b/src/legacy/ui/public/_index.scss index aaed52f8b120a9..b36e62297cc235 100644 --- a/src/legacy/ui/public/_index.scss +++ b/src/legacy/ui/public/_index.scss @@ -9,7 +9,6 @@ // kbnChart__legend-isLoading @import './accessibility/index'; -@import './chrome/index'; @import './directives/index'; @import './error_auto_create_index/index'; @import './error_url_overflow/index'; diff --git a/src/legacy/ui/public/chrome/_index.scss b/src/legacy/ui/public/chrome/_index.scss deleted file mode 100644 index 7e6c3ebaccc5cc..00000000000000 --- a/src/legacy/ui/public/chrome/_index.scss +++ /dev/null @@ -1,3 +0,0 @@ -@import './variables'; - -@import './directives/index'; diff --git a/src/legacy/ui/public/chrome/_variables.scss b/src/legacy/ui/public/chrome/_variables.scss deleted file mode 100644 index 5097fe4c9bfae1..00000000000000 --- a/src/legacy/ui/public/chrome/_variables.scss +++ /dev/null @@ -1,4 +0,0 @@ -$kbnGlobalNavClosedWidth: 53px; -$kbnGlobalNavOpenWidth: 180px; -$kbnGlobalNavLogoHeight: 70px; -$kbnGlobalNavAppIconHeight: $euiSizeXXL + $euiSizeXS; diff --git a/src/legacy/ui/public/chrome/directives/_index.scss b/src/legacy/ui/public/chrome/directives/_index.scss deleted file mode 100644 index 4d00b022791168..00000000000000 --- a/src/legacy/ui/public/chrome/directives/_index.scss +++ /dev/null @@ -1 +0,0 @@ -@import './kbn_chrome'; diff --git a/src/legacy/ui/public/chrome/directives/_kbn_chrome.scss b/src/legacy/ui/public/chrome/directives/_kbn_chrome.scss deleted file mode 100644 index b29a83848d2913..00000000000000 --- a/src/legacy/ui/public/chrome/directives/_kbn_chrome.scss +++ /dev/null @@ -1,46 +0,0 @@ -/** - * stretch the root element of the Kibana application to set the base-size that - * flexed children should keep. Only works when paired with root styles applied - * by core service from new platform - */ -// SASSTODO: Naming here is too embedded and high up that changing them could cause major breaks -#kibana-body { - overflow-x: hidden; - min-height: 100%; -} - -.app-wrapper { - display: flex; - flex-flow: column nowrap; - position: absolute; - left: $kbnGlobalNavClosedWidth; - top: 0; - right: 0; - bottom: 0; - z-index: 5; - margin: 0 auto; - - /** - * 1. Dirty, but we need to override the .kbnGlobalNav-isOpen state - * when we're looking at the log-in screen. - */ - &.hidden-chrome { - left: 0 !important; /* 1 */ - } - - .navbar-right { - margin-right: 0; - } -} - -.app-wrapper-panel { - display: flex; - flex-grow: 1; - flex-shrink: 0; - flex-basis: auto; - flex-direction: column; - - > * { - flex-shrink: 0; - } -} diff --git a/src/legacy/ui/ui_render/bootstrap/template.js.hbs b/src/legacy/ui/ui_render/bootstrap/template.js.hbs index 2d0792cb78c2af..ffabc6a33b61fb 100644 --- a/src/legacy/ui/ui_render/bootstrap/template.js.hbs +++ b/src/legacy/ui/ui_render/bootstrap/template.js.hbs @@ -68,44 +68,17 @@ if (window.__kbnStrictCsp__ && window.__kbnCspNotEnforced__) { }); } - // When rendering in Kibana Platform mode, only serve the assets needed by Core. - if ('{{appId}}' === 'core') { - load([ - {{#each sharedJsDepFilenames}} - '{{../regularBundlePath}}/kbn-ui-shared-deps/{{this}}', - {{/each}} - '{{regularBundlePath}}/kbn-ui-shared-deps/{{sharedJsFilename}}', - '{{regularBundlePath}}/plugin/kibanaUtils/kibanaUtils.plugin.js', - '{{regularBundlePath}}/plugin/esUiShared/esUiShared.plugin.js', - '{{regularBundlePath}}/plugin/kibanaReact/kibanaReact.plugin.js', - '{{regularBundlePath}}/core/core.entry.js', - {{#each styleSheetPaths}} - '{{this}}', - {{/each}} - ]); - return; - } - load([ - {{#each sharedJsDepFilenames}} - '{{../regularBundlePath}}/kbn-ui-shared-deps/{{this}}', - {{/each}} - '{{regularBundlePath}}/kbn-ui-shared-deps/{{sharedJsFilename}}', - '{{dllBundlePath}}/vendors_runtime.bundle.dll.js', - {{#each dllJsChunks}} + {{#each jsDependencies}} '{{this}}', {{/each}} - '{{regularBundlePath}}/commons.bundle.js', - {{!-- '{{regularBundlePath}}/plugin:data/data.plugin.js', --}} - '{{regularBundlePath}}/plugin:kibanaUtils/kibanaUtils.plugin.js', - '{{regularBundlePath}}/plugin:esUiShared/esUiShared.plugin.js', - '{{regularBundlePath}}/plugin:kibanaReact/kibanaReact.plugin.js' ], function () { load([ - '{{regularBundlePath}}/{{appId}}.bundle.js', + '{{entryBundle}}', {{#each styleSheetPaths}} '{{this}}', {{/each}} - ]); - }; + ]); + }); + } } diff --git a/src/legacy/ui/ui_render/ui_render_mixin.js b/src/legacy/ui/ui_render/ui_render_mixin.js index 0912d8683fc485..e4813bd4c56def 100644 --- a/src/legacy/ui/ui_render/ui_render_mixin.js +++ b/src/legacy/ui/ui_render/ui_render_mixin.js @@ -103,8 +103,9 @@ export function uiRenderMixin(kbnServer, server, config) { const dllJsChunks = DllCompiler.getRawDllConfig().chunks.map( chunk => `${dllBundlePath}/vendors${chunk}.bundle.dll.js` ); + const styleSheetPaths = [ - ...dllStyleChunks, + ...(isCore ? [] : dllStyleChunks), `${basePath}/bundles/kbn-ui-shared-deps/${UiSharedDeps.baseCssDistFilename}`, ...(darkMode ? [ @@ -115,29 +116,50 @@ export function uiRenderMixin(kbnServer, server, config) { `${basePath}/bundles/kbn-ui-shared-deps/${UiSharedDeps.lightCssDistFilename}`, `${basePath}/node_modules/@kbn/ui-framework/dist/kui_light.css`, ]), - `${regularBundlePath}/${darkMode ? 'dark' : 'light'}_theme.style.css`, - `${regularBundlePath}/commons.style.css`, - ...(!isCore ? [`${regularBundlePath}/${app.getId()}.style.css`] : []), - ...kbnServer.uiExports.styleSheetPaths - .filter(path => path.theme === '*' || path.theme === (darkMode ? 'dark' : 'light')) - .map(path => - path.localPath.endsWith('.scss') - ? `${basePath}/built_assets/css/${path.publicPath}` - : `${basePath}/${path.publicPath}` - ) - .reverse(), + ...(isCore + ? [] + : [ + `${regularBundlePath}/${darkMode ? 'dark' : 'light'}_theme.style.css`, + `${regularBundlePath}/commons.style.css`, + `${regularBundlePath}/${app.getId()}.style.css`, + ...kbnServer.uiExports.styleSheetPaths + .filter( + path => path.theme === '*' || path.theme === (darkMode ? 'dark' : 'light') + ) + .map(path => + path.localPath.endsWith('.scss') + ? `${basePath}/built_assets/css/${path.publicPath}` + : `${basePath}/${path.publicPath}` + ) + .reverse(), + ]), + ]; + + const jsDependencies = [ + ...UiSharedDeps.jsDepFilenames.map( + filename => `${regularBundlePath}/kbn-ui-shared-deps/${filename}` + ), + `${regularBundlePath}/kbn-ui-shared-deps/${UiSharedDeps.jsFilename}`, + ...(isCore + ? [] + : [ + `${dllBundlePath}/vendors_runtime.bundle.dll.js`, + ...dllJsChunks, + `${regularBundlePath}/commons.bundle.js`, + ]), + `${regularBundlePath}/plugin:kibanaUtils/kibanaUtils.plugin.js`, + `${regularBundlePath}/plugin:esUiShared/esUiShared.plugin.js`, + `${regularBundlePath}/plugin:kibanaReact/kibanaReact.plugin.js`, ]; const bootstrap = new AppBootstrap({ templateData: { - appId: isCore ? 'core' : app.getId(), - regularBundlePath, - dllBundlePath, - dllJsChunks, - styleSheetPaths, - sharedJsFilename: UiSharedDeps.jsFilename, - sharedJsDepFilenames: UiSharedDeps.jsDepFilenames, darkMode, + jsDependencies, + styleSheetPaths, + entryBundle: isCore + ? `${regularBundlePath}/core/core.entry.js` + : `${regularBundlePath}/${app.getId()}.bundle.js`, }, }); diff --git a/src/optimize/bundles_route/bundles_route.js b/src/optimize/bundles_route/bundles_route.js index 24bc11eede7f48..f15760795391a2 100644 --- a/src/optimize/bundles_route/bundles_route.js +++ b/src/optimize/bundles_route/bundles_route.js @@ -17,11 +17,12 @@ * under the License. */ -import { isAbsolute, extname, resolve, join } from 'path'; +import { isAbsolute, extname, join } from 'path'; import LruCache from 'lru-cache'; import * as UiSharedDeps from '@kbn/ui-shared-deps'; import { createDynamicAssetResponse } from './dynamic_asset_response'; import { assertIsNpUiPluginPublicDirs } from '../np_ui_plugin_public_dirs'; +import { fromRoot } from '../../core/server/utils'; /** * Creates the routes that serves files from `bundlesPath` or from @@ -88,7 +89,7 @@ export function createBundlesRoute({ buildRouteForBundles( `${basePublicPath}/bundles/core/`, `/bundles/core/`, - resolve(__dirname, join('..', '..', 'core', 'public', 'target', 'public')), + fromRoot(join('src', 'core', 'target', 'public')), fileHashCache ), buildRouteForBundles( diff --git a/x-pack/plugins/graph/public/angular/templates/_graph.scss b/x-pack/plugins/graph/public/angular/templates/_graph.scss index e6bd4693d1e9b8..4ba65e7ec6b96a 100644 --- a/x-pack/plugins/graph/public/angular/templates/_graph.scss +++ b/x-pack/plugins/graph/public/angular/templates/_graph.scss @@ -1,5 +1,3 @@ -@import 'src/legacy/ui/public/chrome/variables'; - @mixin gphSvgText() { font-family: $euiFontFamily; font-size: $euiSizeS;