[Workplace Search] Convert Sources pages to new page template (+ personal dashboard) by cee-chen · Pull Request #102592 · elastic/kibana

cee-chen · 2021-06-18T02:23:03Z

Summary

Follow up to #102170 - this is the last PR for converting a chunk of Workplace Search views to the new KibanaPageTemplate. It's the largest and most complex one since Sources is already the most complex set of views, and requires accounting for the Personal Dashboard layout + updating that as well. As always, follow along by commit and disable whitespace diffs, etc.

Screencaps

Organization Sources

Personal Dashboard

(Not sure why this one spent so much time loading or had an error on the content sources page...maybe because I'd just added the slack connector?)

QA

⚠️ I need QA help on this one! I'm fairly sure I QA'd everything, but it's strongly possible I missed something from just not knowing enough about Workplace Search. Please pull this down locally and check to make sure:

all pages look visually fine and load as expected
empty states look fine as well as filled states
document titles+breadcrumbs update as expected
reloading/refreshing the app on all pages works and doesn't cause strange behavior or crashes due to (e.g.) incorrect logic mount order

Checklist

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)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

- Remove references to enterpriseSearchLayout CSS (which will be removed in an upcoming PR) - Prefer to lean more heavily on default EuiPage props/CSS/etc. - Handle conditional sidebar logic in this layout rather than passing it in as a prop - Update props & DRY concerns to more closely match WorkplaceSearchPageTemplate - e.g. isLoading & pageChrome (mostly for document titles) - make FlashMessage and readOnlyMode work OOTB w/o props)

+ update PrivateSourcesSidebar to use EuiSIdeNav

- removing wrapping layouts, flash messages, chrome/telemetry

- Remove license callout, page header, and page chrome/telemetry - NOTE: The early page isLoading behavior (lines 51-) is required to prevent a flash of a completely empty page (instead of preserving the layout/side nav while loading). We cannot let the page fall through to the route because some routes are conditionally rendered based on isCustomSource. - FWIW: App Search has a similar isLoading early return with its Engine sub nav, and also a similar AnalyticsLayout for DRYing out repeated concerns/UI elements within Analytics subroutes.

- Mostly removing isLoading tests - NOTE: Schema page could *possibly* use the new isEmptyState/emptyState page template props, but would need some layout reshuffling

- Opted to give these pages their own conditional layout logic - this could possibly be DRY'd out - There is possibly extra cleanup here on this file that could have been done (e.g. empty state, titles, etc.) in light of the new templates - but I didn't want to spend extra time here and went with creating as few diffs as possible

…page templates + fix Link to EuiButtonTo on Organization Sources view

+ add related KibanaLogic branch coverage

...terprise_search/public/applications/workplace_search/views/content_sources/source_router.tsx

...arch/public/applications/workplace_search/views/content_sources/components/source_layout.tsx

...arch/public/applications/workplace_search/views/content_sources/components/schema/schema.tsx

...pplications/workplace_search/views/content_sources/components/add_source/add_source_list.tsx

...search/public/applications/workplace_search/views/account_settings/account_settings.test.tsx

x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.test.ts

...ns/workplace_search/components/layout/personal_dashboard_sidebar/private_sources_sidebar.tsx

...s/workplace_search/components/layout/personal_dashboard_layout/personal_dashboard_layout.tsx

x-pack/plugins/enterprise_search/public/applications/workplace_search/index.tsx

...arch/public/applications/workplace_search/views/content_sources/components/schema/schema.tsx

...e_search/public/applications/workplace_search/views/content_sources/organization_sources.tsx

x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.test.ts

...search/public/applications/workplace_search/views/account_settings/account_settings.test.tsx

x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.test.ts

…Sources pages

x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.test.ts

yakhinvadim · 2021-06-18T18:46:01Z

@constancecchen I finished the QA and everything except already posted comments looks great! Thank you so much for handling this update! Looking forward to the last PR! 🎉

…crumbs - `exact` is required to make the parent schemas/ not gobble schema/{errorId} - added bonus breadcrumb for nicer schema navigation UX - No tests need to update AFAICT

cee-chen · 2021-06-18T18:59:10Z

Thanks so much for the super thorough QA Vadim!!! Just to confirm, no issues on the personal dashboard either right?

If not, I guess we get to wait for EUI! 🎉

yakhinvadim

Just to confirm, no issues on the personal dashboard either right?

Yep, no issues!

Approving since all comments are resolved. 💃

kibanamachine · 2021-06-22T00:42:13Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`enterpriseSearch`	1454	1455	+1

Async chunks

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

id	before	after	diff
`enterpriseSearch`	2.1MB	2.1MB	-682.0B

History

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

kibanamachine · 2021-06-22T00:44:37Z

💔 Backport failed

Status	Branch	Result
❌	7.x	Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 102592

…onal dashboard) (elastic#102592) * Refactor PersonalDashboardLayout to more closely match new page template - Remove references to enterpriseSearchLayout CSS (which will be removed in an upcoming PR) - Prefer to lean more heavily on default EuiPage props/CSS/etc. - Handle conditional sidebar logic in this layout rather than passing it in as a prop - Update props & DRY concerns to more closely match WorkplaceSearchPageTemplate - e.g. isLoading & pageChrome (mostly for document titles) - make FlashMessage and readOnlyMode work OOTB w/o props) * Convert Source subnav to EuiSideNav format + update PrivateSourcesSidebar to use EuiSIdeNav * Update routers - removing wrapping layouts, flash messages, chrome/telemetry * Refactor SourceRouter into shared layout component - Remove license callout, page header, and page chrome/telemetry - NOTE: The early page isLoading behavior (lines 51-) is required to prevent a flash of a completely empty page (instead of preserving the layout/side nav while loading). We cannot let the page fall through to the route because some routes are conditionally rendered based on isCustomSource. - FWIW: App Search has a similar isLoading early return with its Engine sub nav, and also a similar AnalyticsLayout for DRYing out repeated concerns/UI elements within Analytics subroutes. * Convert all single source views to new page template - Mostly removing isLoading tests - NOTE: Schema page could *possibly* use the new isEmptyState/emptyState page template props, but would need some layout reshuffling * Convert Add Source pages to conditional page templates - Opted to give these pages their own conditional layout logic - this could possibly be DRY'd out - There is possibly extra cleanup here on this file that could have been done (e.g. empty state, titles, etc.) in light of the new templates - but I didn't want to spend extra time here and went with creating as few diffs as possible * Convert separate Organization Sources & Private Sources views to new page templates + fix Link to EuiButtonTo on Organization Sources view * Update Account Settings with personal layout + write tests + add related KibanaLogic branch coverage * [UX feedback] Do not render page headers while loading on Overview & Sources pages * [PR feedback] Breadcrumb errors/fallbacks * [Proposal] Update schema errors routing to better work with nav/breadcrumbs - `exact` is required to make the parent schemas/ not gobble schema/{errorId} - added bonus breadcrumb for nicer schema navigation UX - No tests need to update AFAICT * Ignore Typescript error on soon-to-come EUI prop # Conflicts: # x-pack/plugins/enterprise_search/public/applications/workplace_search/views/content_sources/components/source_settings.tsx

…onal dashboard) (#102592) (#102843) * Refactor PersonalDashboardLayout to more closely match new page template - Remove references to enterpriseSearchLayout CSS (which will be removed in an upcoming PR) - Prefer to lean more heavily on default EuiPage props/CSS/etc. - Handle conditional sidebar logic in this layout rather than passing it in as a prop - Update props & DRY concerns to more closely match WorkplaceSearchPageTemplate - e.g. isLoading & pageChrome (mostly for document titles) - make FlashMessage and readOnlyMode work OOTB w/o props) * Convert Source subnav to EuiSideNav format + update PrivateSourcesSidebar to use EuiSIdeNav * Update routers - removing wrapping layouts, flash messages, chrome/telemetry * Refactor SourceRouter into shared layout component - Remove license callout, page header, and page chrome/telemetry - NOTE: The early page isLoading behavior (lines 51-) is required to prevent a flash of a completely empty page (instead of preserving the layout/side nav while loading). We cannot let the page fall through to the route because some routes are conditionally rendered based on isCustomSource. - FWIW: App Search has a similar isLoading early return with its Engine sub nav, and also a similar AnalyticsLayout for DRYing out repeated concerns/UI elements within Analytics subroutes. * Convert all single source views to new page template - Mostly removing isLoading tests - NOTE: Schema page could *possibly* use the new isEmptyState/emptyState page template props, but would need some layout reshuffling * Convert Add Source pages to conditional page templates - Opted to give these pages their own conditional layout logic - this could possibly be DRY'd out - There is possibly extra cleanup here on this file that could have been done (e.g. empty state, titles, etc.) in light of the new templates - but I didn't want to spend extra time here and went with creating as few diffs as possible * Convert separate Organization Sources & Private Sources views to new page templates + fix Link to EuiButtonTo on Organization Sources view * Update Account Settings with personal layout + write tests + add related KibanaLogic branch coverage * [UX feedback] Do not render page headers while loading on Overview & Sources pages * [PR feedback] Breadcrumb errors/fallbacks * [Proposal] Update schema errors routing to better work with nav/breadcrumbs - `exact` is required to make the parent schemas/ not gobble schema/{errorId} - added bonus breadcrumb for nicer schema navigation UX - No tests need to update AFAICT * Ignore Typescript error on soon-to-come EUI prop # Conflicts: # x-pack/plugins/enterprise_search/public/applications/workplace_search/views/content_sources/components/source_settings.tsx

cee-chen added 8 commits June 17, 2021 18:11

Convert Source subnav to EuiSideNav format

b4ebfa4

+ update PrivateSourcesSidebar to use EuiSIdeNav

Update routers

1fffcf6

- removing wrapping layouts, flash messages, chrome/telemetry

Convert all single source views to new page template

7bb4891

- Mostly removing isLoading tests - NOTE: Schema page could *possibly* use the new isEmptyState/emptyState page template props, but would need some layout reshuffling

Convert separate Organization Sources & Private Sources views to new …

7f42518

…page templates + fix Link to EuiButtonTo on Organization Sources view

Update Account Settings with personal layout + write tests

6c305e5

+ add related KibanaLogic branch coverage

cee-chen added release_note:skip Skip the PR/issue when compiling release notes v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 18, 2021

cee-chen requested review from a team June 18, 2021 02:23

cee-chen commented Jun 18, 2021

View reviewed changes

...ns/workplace_search/components/layout/personal_dashboard_sidebar/private_sources_sidebar.tsx Show resolved Hide resolved

cee-chen commented Jun 18, 2021

View reviewed changes

...s/workplace_search/components/layout/personal_dashboard_layout/personal_dashboard_layout.tsx Show resolved Hide resolved

This comment has been minimized.

Sign in to view

yakhinvadim reviewed Jun 18, 2021

View reviewed changes

x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.test.ts Show resolved Hide resolved

yakhinvadim reviewed Jun 18, 2021

View reviewed changes

x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.test.ts Show resolved Hide resolved

[UX feedback] Do not render page headers while loading on Overview & …

8bea24e

…Sources pages

yakhinvadim reviewed Jun 18, 2021

View reviewed changes

x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.test.ts Show resolved Hide resolved

[PR feedback] Breadcrumb errors/fallbacks

d77ee41

[Proposal] Update schema errors routing to better work with nav/bread…

b6a62b6

…crumbs - `exact` is required to make the parent schemas/ not gobble schema/{errorId} - added bonus breadcrumb for nicer schema navigation UX - No tests need to update AFAICT

yakhinvadim approved these changes Jun 18, 2021

View reviewed changes

This was referenced Jun 19, 2021

[App Search] Convert Engines overview & engine creation to new page template #102677

Merged

[App Search] Convert Engine subnav and Engine Overview pages to new page template #102679

Merged

Merge branch 'master' into kibana-page-template-ws-4

96ef51c

cee-chen added 2 commits June 21, 2021 14:14

Ignore Typescript error on soon-to-come EUI prop

57dcd76

Fix route test - caused by schema reindex URL change

baf2ab6

cee-chen enabled auto-merge (squash) June 21, 2021 21:17

One more unit test route update

e562f63

cee-chen merged commit 138bd0d into elastic:master Jun 22, 2021

cee-chen deleted the kibana-page-template-ws-4 branch June 22, 2021 01:21

cee-chen mentioned this pull request Jun 22, 2021

[7.x] [Workplace Search] Convert Sources pages to new page template (+ personal dashboard) (#102592) #102843

Merged

cee-chen mentioned this pull request Jun 23, 2021

[Enterprise Search] Support active nav links that have both subnav & non-subnav child routes #103036

Merged

1 task

Conversation

cee-chen commented Jun 18, 2021 • edited by yakhinvadim Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Screencaps

Organization Sources

Personal Dashboard

QA

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yakhinvadim commented Jun 18, 2021

Uh oh!

cee-chen commented Jun 18, 2021

Uh oh!

yakhinvadim left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jun 22, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

Uh oh!

kibanamachine commented Jun 22, 2021

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cee-chen commented Jun 18, 2021 •

edited by yakhinvadim

Loading

yakhinvadim left a comment •

edited

Loading