Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate home app #58030

Merged
merged 20 commits into from
Mar 4, 2020
Merged

Migrate home app #58030

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
38f6cc4
remove injected reference from home app
flash1293 Feb 18, 2020
34eb369
fix home plugin mocks
flash1293 Feb 18, 2020
c47b753
Merge remote-tracking branch 'upstream/master' into de-cloud-home
flash1293 Feb 19, 2020
ac80731
Merge branch 'de-cloud-home' into migrate-home
flash1293 Feb 19, 2020
80a5194
migrate home plugin
flash1293 Feb 19, 2020
bd45e74
Merge remote-tracking branch 'upstream/master' into migrate-home
flash1293 Feb 19, 2020
a49d768
fix test failures
flash1293 Feb 19, 2020
b8a23f9
Merge remote-tracking branch 'upstream/master' into migrate-home
flash1293 Feb 20, 2020
d508e85
fix type errors
flash1293 Feb 20, 2020
335e587
Merge remote-tracking branch 'upstream/master' into migrate-home
flash1293 Feb 21, 2020
d029823
fix i18n ids
flash1293 Feb 21, 2020
2722e0e
Merge remote-tracking branch 'upstream/master' into migrate-home
flash1293 Feb 21, 2020
a122c8f
nomve home styles back for now
flash1293 Feb 21, 2020
9363c5a
Merge branch 'master' into migrate-home
elasticmachine Feb 24, 2020
17b50f9
Merge remote-tracking branch 'upstream/master' into migrate-home
flash1293 Mar 2, 2020
7a68c39
fix migrated home app
flash1293 Mar 2, 2020
6b5f0e6
Merge branch 'migrate-home' of github.com:flash1293/kibana into migra…
flash1293 Mar 2, 2020
02340e8
import scss from new platform
flash1293 Mar 2, 2020
10211ae
Merge remote-tracking branch 'upstream/master' into migrate-home
flash1293 Mar 3, 2020
9ef44aa
revert new platform scss import
flash1293 Mar 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/legacy/core_plugins/kibana/public/.eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ module.exports = {
{
basePath: path.resolve(__dirname, '../../../../../'),
zones: topLevelRestricedZones.concat(
buildRestrictedPaths(['visualize', 'discover', 'dashboard', 'devTools', 'home'])
buildRestrictedPaths(['visualize', 'discover', 'dashboard', 'devTools'])
),
},
],
Expand Down
1 change: 0 additions & 1 deletion src/legacy/core_plugins/kibana/public/home/_index.scss
View file
Open in desktop

This file was deleted.

42 changes: 0 additions & 42 deletions src/legacy/core_plugins/kibana/public/home/np_ready/components/home.test.mocks.ts
View file
Open in desktop

This file was deleted.

105 changes: 0 additions & 105 deletions src/legacy/core_plugins/kibana/public/home/plugin.ts
View file
Open in desktop

This file was deleted.

6 changes: 3 additions & 3 deletions src/legacy/core_plugins/kibana/public/index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@
// Discover styles
@import 'discover/index';

// Home styles
@import './home/index';

// Visualize styles
@import './visualize/index';
// Has to come after visualize because of some
// bad cascading in the Editor layout
@import 'src/legacy/ui/public/vis/index';

// Home styles
@import '../../../../plugins/home/public/application/index';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you import this file directly in the JS in plugins/home/public/ instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

And also then rename it from _index.scss to index.scss

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally included this but reverted because the scss is depending on legacy mixins in “ui/styles”. Importing from a module in the new platform gave me an error. The platform team is owning the topic and is tracking this separately, as soon as it’s moved the entry-point can be moved over just as you suggest.

Copy link
Contributor

Choose a reason for hiding this comment

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

You could then just import the mixin dependencies directly in the index.scss file so as to unblock moving this to the JS file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It’s not complaining about the mixin itself (somehow it’s pulling that in implicitly), but about an asset used within the mixin: https://github.com/elastic/kibana/blob/master/src/legacy/ui/public/styles/_mixins.scss#L92

That’s the line failing. As it’s going to get migrated soon I didn’t dig any deeper , but maybe you know a better way around the problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

What does the error say?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I wonder then why yours would be the only implementation that complains if it's not the one pulling the file directly...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't reproduce the problem locally anymore and the Jenkins build with this failure is already cleaned up - I committed the change again, if it works even better.

Also, I wonder then why yours would be the only implementation that complains if it's not the one pulling the file directly...

There is no usage of that specific mixin from within the new platform so far, it's only consumed the legacy way:

x-pack/plugins/spaces/public/space_selector/_space_selector.scss
2:  @include kibanaFullScreenGraphics;

x-pack/legacy/plugins/security/public/components/authentication_state_page/_authentication_state_page.scss
2:  @include kibanaFullScreenGraphics;

x-pack/legacy/plugins/security/public/views/login/components/login_page/_login_page.scss
2:  @include kibanaFullScreenGraphics;

src/plugins/home/public/application/components/_welcome.scss
2:  @include kibanaFullScreenGraphics($euiZLevel6);

Spaces selector looks like it's imported in the new platform, but it's still used in the old way eventually: x-pack/legacy/plugins/spaces/public/index.scss

Copy link
Contributor

Choose a reason for hiding this comment

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

I just pulled down your branch, ran it locally, didn't get any compile or console errors and the graphics rendered properly in the Welcome page. So... 👍 looks like all is good with the import in JS?


// Management styles
@import './management/index';

Expand Down
1 change: 0 additions & 1 deletion src/legacy/core_plugins/kibana/public/kibana.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import 'uiExports/interpreter';

import 'ui/autoload/all';
import 'ui/kbn_top_nav';
import './home';
import './discover/legacy';
import './visualize/legacy';
import './dashboard/legacy';
Expand Down
15 changes: 3 additions & 12 deletions src/legacy/ui/public/new_platform/new_platform.karma_mock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,9 @@ export const npSetup = {
config: {
disableWelcomeScreen: false,
},
tutorials: {
setVariable: sinon.fake(),
},
},
charts: {
theme: {
Expand Down Expand Up @@ -380,18 +383,6 @@ export const npStart = {
getTriggerActions: sinon.fake(),
getTriggerCompatibleActions: sinon.fake(),
},
home: {
featureCatalogue: {
get: sinon.fake(),
register: sinon.fake(),
},
environment: {
get: sinon.fake(),
},
config: {
disableWelcomeScreen: false,
},
},
navigation: {
ui: {
TopNavMenu: mockComponent,
Expand Down
3 changes: 1 addition & 2 deletions src/legacy/ui/public/new_platform/new_platform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
import { ChartsPluginSetup, ChartsPluginStart } from '../../../../plugins/charts/public';
import { DevToolsSetup, DevToolsStart } from '../../../../plugins/dev_tools/public';
import { KibanaLegacySetup, KibanaLegacyStart } from '../../../../plugins/kibana_legacy/public';
import { HomePublicPluginSetup, HomePublicPluginStart } from '../../../../plugins/home/public';
import { HomePublicPluginSetup } from '../../../../plugins/home/public';
import { SharePluginSetup, SharePluginStart } from '../../../../plugins/share/public';
import {
AdvancedSettingsSetup,
Expand Down Expand Up @@ -72,7 +72,6 @@ export interface PluginsStart {
data: ReturnType<DataPlugin['start']>;
embeddable: IEmbeddableStart;
expressions: ReturnType<ExpressionsPlugin['start']>;
home: HomePublicPluginStart;
inspector: InspectorStart;
uiActions: UiActionsStart;
navigation: NavigationPublicPluginStart;
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/home/kibana.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"version": "kibana",
"server": true,
"ui": true,
"optionalPlugins": ["usage_collection"]
"requiredPlugins": ["data", "kibanaLegacy"],
"optionalPlugins": ["usage_collection", "telemetry"]
}
1 change: 1 addition & 0 deletions src/plugins/home/public/application/_index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import 'components/index';
4 changes: 2 additions & 2 deletions ...bana/public/home/np_ready/application.tsx → ...s/home/public/application/application.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import { render, unmountComponentAtNode } from 'react-dom';
import { i18n } from '@kbn/i18n';
// @ts-ignore
import { HomeApp } from './components/home_app';
import { getServices } from '../kibana_services';
import { getServices } from './kibana_services';

export const renderApp = async (element: HTMLElement) => {
const homeTitle = i18n.translate('kbn.home.breadcrumbs.homeTitle', { defaultMessage: 'Home' });
const homeTitle = i18n.translate('home.breadcrumbs.homeTitle', { defaultMessage: 'Home' });
const { featureCatalogue, chrome } = getServices();

// all the directories could be get in "start" phase of plugin after all of the legacy plugins will be moved to a NP
Expand Down
Loading