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.

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

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

Merged
merged 1 commit into from
Jun 22, 2021

Conversation

cee-chen
Copy link
Contributor

Backports the following commits to 7.x:

…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
@cee-chen cee-chen enabled auto-merge (squash) June 22, 2021 01:26
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
enterpriseSearch 1449 1450 +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

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

@cee-chen cee-chen merged commit e265826 into elastic:7.x Jun 22, 2021
@cee-chen cee-chen deleted the backport/7.x/pr-102592 branch June 22, 2021 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants