[SharedUX] Migrate PageTemplate > NoDataPage > SolutionNav #128277

majagrubic · 2022-03-22T15:16:31Z

Summary

This is continuation of migrating PageTemplate components. The components have been migrated almost as-is, including .sass files, as they use a lot of mixins that haven't been transformed into appropriate hooks. Some comments inline.

Depends on: #128386

Checklist

Delete any items that are not applicable to this PR.

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
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
~~- [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list~~
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

majagrubic · 2022-03-22T15:17:58Z

packages/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav.scss

@@ -0,0 +1,30 @@
+$euiSideNavEmphasizedBackgroundColor: transparentize($euiColorLightShade, .7);
+@import '@elastic/eui/src/components/side_nav/mixins';


I don't think this should be imported like this. Is it acceptable to just copy the relevant styling as js constants and use them here?

The mixin was created by EUI specifically to be used in multiple Elastic products, we don't want to duplicate styles because then they'll get out of sync. This method of importing has been fine and eventually will get converted to a JS hook. So it's ok for now.

packages/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav.tsx

majagrubic · 2022-03-22T15:19:33Z

packages/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav_avatar.scss

+
+  &--xxl {
+    @include euiBottomShadowMedium;
+    @include size(100px);


What is this mixin for? I haven't been able to find its appropriate definition in eui codebase.

majagrubic · 2022-03-25T07:17:29Z

@elasticmachine merge upstream

kibanamachine · 2022-03-25T07:17:41Z

expected head sha didn’t match current head ref.

majagrubic · 2022-03-28T10:05:05Z

@elasticmachine merge upstream

majagrubic · 2022-03-30T11:25:15Z

@elasticmachine merge upstream

cchaos

The story for the navigation toggle is very odd, is there any way you make it seem more appropriate? Just by going there I don't understand why it's so big, what it's for, or what it's supposed to do...

Maybe it just doesn't make sense for it to have it's own story at all since it does nothing outside of the context of the actual solution nav. You could instead add more story controls to the sidenav based on isOpenOnDesktop and/or onCollapse.

packages/kbn-shared-ux-components/src/page_template/solution_nav/assets/texture.svg

cchaos · 2022-03-30T19:26:23Z

packages/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav.scss

@@ -0,0 +1,30 @@
+$euiSideNavEmphasizedBackgroundColor: transparentize($euiColorLightShade, .7);
+@import '@elastic/eui/src/components/side_nav/mixins';


The mixin was created by EUI specifically to be used in multiple Elastic products, we don't want to duplicate styles because then they'll get out of sync. This method of importing has been fine and eventually will get converted to a JS hook. So it's ok for now.

packages/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav.stories.tsx

...ges/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav_collapse_button.tsx

packages/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav.tsx

packages/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav.stories.tsx

majagrubic · 2022-03-31T10:57:45Z

@elasticmachine merge upstream

majagrubic · 2022-03-31T17:20:46Z

@elasticmachine merge upstream

cchaos

Last few thoughts

cchaos · 2022-03-31T19:19:20Z

packages/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav.tsx

+  name,
+  icon,
+  items,
+  isOpenOnDesktop = true,


We can try it this way, but I'm thinking there was a reason the default needed to be false when it gets controlled by the page side bar.

cchaos · 2022-03-31T19:23:36Z

packages/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav.stories.tsx

+  },
+  icon: {
+    control: { type: 'radio' },
+    options: ['logoObservability', 'logoSecurity'],


Ah sorry, I thought the name was hooked up to pass all the way through to the Avatar, but it doesn't. So let's also add the Kibana option and make selected by default.

Suggested change

options: ['logoObservability', 'logoSecurity'],

options: ['logoKibana', 'logoObservability', 'logoSecurity'],

defaultValue: 'logoKibana',

packages/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav.stories.tsx

…into page-template-3

kibana-ci · 2022-04-01T10:29:13Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dataViewEditor`	140	153	+13
`spaces`	251	264	+13
total			+26

Async chunks

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

id	before	after	diff
`dataViewEditor`	142.8KB	154.0KB	+11.2KB
`spaces`	257.5KB	268.6KB	+11.1KB
total			+22.2KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`@kbn/shared-ux-components`	3	4	+1

Page load bundle

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

id	before	after	diff
`dataViewEditor`	10.6KB	10.6KB	-1.0B
`spaces`	20.4KB	20.4KB	+1.0B
total			-0.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/shared-ux-components`	28	32	+4

async chunk count

id	before	after	diff
`dataViewEditor`	6	7	+1
`spaces`	16	17	+1
total			+2

History

💔 Build #35291 failed f2f082f
💔 Build #35267 failed 41ffd07
💚 Build #35182 succeeded 1b5ef2d
💔 Build #35146 failed 3c1179c
💚 Build #34823 succeeded b3b98b5

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

cchaos

Thanks for the changes. LGTM!

majagrubic commented Mar 22, 2022

View reviewed changes

packages/kbn-shared-ux-components/src/page_template/solution_nav/solution_nav.tsx Show resolved Hide resolved

majagrubic commented Mar 22, 2022

View reviewed changes

majagrubic force-pushed the page-template-3 branch from 87fe426 to 505654d Compare March 22, 2022 19:26

majagrubic added Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) release_note:skip Skip the PR/issue when compiling release notes v8.3.0 labels Mar 23, 2022

majagrubic marked this pull request as ready for review March 23, 2022 13:34

majagrubic requested a review from a team as a code owner March 23, 2022 13:34

majagrubic requested a review from cchaos March 23, 2022 13:34

Maja Grubic added 9 commits March 23, 2022 16:28

[SharedUX] Migrate PageTemplate > SolutionNavAvatar

4c02548

[SharedUX] Migrate PageTemplate > NoDataPage > ActionCards

b0c2032

[SharedUX] Migrate PageTemplate > NoDataPage > SolutionNav

2664372

Updating snapshot

6756004

Fix i18n

95387f2

Fix index.tsx

ab4e3b2

Fix failing test

b4a9f0f

Remove unnecessary export

9b8877b

Change folder structure of solution_nav_avatar

347e401

majagrubic requested review from a team as code owners March 23, 2022 15:35

Maja Grubic added 2 commits March 23, 2022 16:39

[SharedUX] Migrate PageTemplate > NoDataPage > ActionCards

f6d818c

[SharedUX] Migrate PageTemplate > NoDataPage > SolutionNav

30534ea

Maja Grubic added 2 commits March 24, 2022 07:08

Fix SolutionAvatar reference

96859b2

Resolving merge conflicts

b6515ba

Merge branch 'main' into page-template-3

0f9f73d

Maja Grubic added 2 commits March 25, 2022 10:32

Fix failing test

f713bae

Fix failing snapshot

3e772af

Merge branch 'main' into page-template-3

8bf163d

Merge branch 'main' into page-template-3

b3b98b5

cchaos suggested changes Mar 30, 2022

View reviewed changes

Maja Grubic added 3 commits March 31, 2022 11:09

Applying PR comments

329bc92

Updating failing snapshot

5e67940

Extract i18n

3c1179c

Merge branch 'main' into page-template-3

1b5ef2d

majagrubic requested a review from cchaos March 31, 2022 13:00

collapsed > isCollapsed

41ffd07

Merge branch 'main' into page-template-3

f2f082f

cchaos suggested changes Mar 31, 2022

View reviewed changes

Maja Grubic added 2 commits April 1, 2022 10:48

Applying PR comments

973d86a

Merge branch 'page-template-3' of https://github.com/majagrubic/kibana …

4f4c217

…into page-template-3

majagrubic requested a review from cchaos April 1, 2022 12:41

cchaos approved these changes Apr 1, 2022

View reviewed changes

majagrubic merged commit 9709107 into elastic:main Apr 2, 2022

kibanamachine added the backport:skip This commit does not require backporting label Apr 2, 2022

majagrubic deleted the page-template-3 branch April 2, 2022 06:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SharedUX] Migrate PageTemplate > NoDataPage > SolutionNav #128277

[SharedUX] Migrate PageTemplate > NoDataPage > SolutionNav #128277

majagrubic commented Mar 22, 2022 •

edited

Loading

majagrubic Mar 22, 2022

cchaos Mar 30, 2022

majagrubic Mar 22, 2022

majagrubic commented Mar 25, 2022

kibanamachine commented Mar 25, 2022

majagrubic commented Mar 28, 2022

majagrubic commented Mar 30, 2022

cchaos left a comment

cchaos Mar 30, 2022

majagrubic commented Mar 31, 2022

majagrubic commented Mar 31, 2022

cchaos left a comment

cchaos Mar 31, 2022

cchaos Mar 31, 2022

kibana-ci commented Apr 1, 2022

API count

async chunk count

cchaos left a comment

		@@ -0,0 +1,30 @@
		$euiSideNavEmphasizedBackgroundColor: transparentize($euiColorLightShade, .7);
		@import '@elastic/eui/src/components/side_nav/mixins';

	options: ['logoObservability', 'logoSecurity'],
	options: ['logoKibana', 'logoObservability', 'logoSecurity'],
	defaultValue: 'logoKibana',

[SharedUX] Migrate PageTemplate > NoDataPage > SolutionNav #128277

[SharedUX] Migrate PageTemplate > NoDataPage > SolutionNav #128277

Conversation

majagrubic commented Mar 22, 2022 • edited Loading

Summary

Checklist

For maintainers

majagrubic Mar 22, 2022

Choose a reason for hiding this comment

cchaos Mar 30, 2022

Choose a reason for hiding this comment

majagrubic Mar 22, 2022

Choose a reason for hiding this comment

majagrubic commented Mar 25, 2022

kibanamachine commented Mar 25, 2022

majagrubic commented Mar 28, 2022

majagrubic commented Mar 30, 2022

cchaos left a comment

Choose a reason for hiding this comment

cchaos Mar 30, 2022

Choose a reason for hiding this comment

majagrubic commented Mar 31, 2022

majagrubic commented Mar 31, 2022

cchaos left a comment

Choose a reason for hiding this comment

cchaos Mar 31, 2022

Choose a reason for hiding this comment

cchaos Mar 31, 2022

Choose a reason for hiding this comment

kibana-ci commented Apr 1, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Public APIs missing exports

Page load bundle

API count

async chunk count

History

cchaos left a comment

Choose a reason for hiding this comment

majagrubic commented Mar 22, 2022 •

edited

Loading