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

[WIP] Page layouts #24

Closed
wants to merge 9 commits into from
Closed

[WIP] Page layouts #24

wants to merge 9 commits into from

Conversation

cchaos
Copy link
Owner

@cchaos cchaos commented Mar 31, 2021

Starting in my fork as to not scare anyone... yet

With each "stage", I'll add a comment of what changed.

Checklist (specific to this endeavor)

  • Reduce the amount of wrapping divs
    • Make sure it doesn't cause regressions
  • Test out the different types of templates
  • Decide where the service should live (as a consumable or to push up options?)

@cchaos
Copy link
Owner Author

cchaos commented Mar 31, 2021

Part 1: Reduce/Reuse/Recycle

[ ] A. Reduced the number of wrapping elements before getting to actual page content.

It seems that over the years, instead of fixing these layers, more were added. This is trouble for the types of layouts we try to create using flex to stretch the contents. The following is the old vs new rendered DOM:

image

Essentially it boils down to:

1. Reducing the wrappers in the rendering service

  • Removed the .content wrapper
    • There were no styles attached to this element
    • There was a data-test-subj="kibanaChrome" which is targeted by some tests, but I've moved this to the #kbnBody element (one level above). Iffy on whether that was really the appropriate element to move it to or if it needs to be within the service.
  • Removed the extraneous .app-wrapper-panel wrapping element
  • Removed the <AppContainer /> wrapper which just passed down the applicationClasses.
    • I've moved these classes to be applied to the <AppWrapper /> instead

The result of which is

ReactDOM.render(
<I18nProvider>
<>
{chromeHeader}
<div id="globalBannerList">{bannerComponent}</div>
<AppWrapper
chromeVisible$={chrome.getIsVisible$()}
classes$={chrome.getApplicationClasses$()}
>
{appComponent}
</AppWrapper>
</>
</I18nProvider>,
targetDomElement
);

2. Added a better and reusable class name for any elements wrapping applications

The AppWrapper component can now be reused by individual applications to ensure proper layout, or they can use the APP_WRAPPER_CLASS exported variable.

The variable is for places like Dashboard still creating elements through the document.

params.element.classList.add(APP_WRAPPER_CLASS);

3. Updated some naming of classes just to be more consistant:

  • #kibana-body -> #kbnChrome
  • .app-wrapper -> .kbnAppWrapper

B. [ ] Created the KibanaPageLayout component

This is essentially a copy/paste from my POC so there will be more work to do on it to fit Kibana specifically. It contains a placeholder for where to add the KQL/time pickers as globals as well as a placeholder for the solutionNav to pass a node.

It's got some of the most commonly used layouts that I've found, but I haven't tested them all yet.

C. [ ] Converted the already shared TableListView to use the KibanaPageLayout

This is currently being reused in the Dashboard, Maps, Canvas, and Visualize listing pages. There's most likely a "bubble up" needed to happen but I didn't get too much further than just replacing the manual EuiPage components for the Kibana specific page layout component.

Screen Shot 2021-03-31 at 16 06 09 PM

---

Conclusion so far

I've looked at different applications with different layouts like Dashboard, Canvas, Lens, Maps, Overview, Stack management and a few solution pages. I haven't seen any regressions (so far).

@cchaos cchaos requested a review from myasonik March 31, 2021 20:05
@cchaos
Copy link
Owner Author

cchaos commented Apr 1, 2021

Part 2: Kibana Overview Pages converted

Mostly just converted to use the KibanaPageLayout and cleaned up code/styles. Nothing functionally or stylistically has changed except for the removal of the style that showed the content crossing the border of the page header.

Screen Shot 2021-04-01 at 09 31 52 AM

@myasonik
Copy link
Collaborator

myasonik commented Apr 2, 2021

Part 3: Initial work on Management pages

Create a system

Because Management pages had page structure split between the Management app and the mounted pages from other plugins, some refactor was necessary in the Management app and some will be needed in each page.

After clearing out some cruft in the Management app, the notable change is that each mounted page now gets a managementPageLayout passed in which is a setup KibanaPageLayout with the ManagementSidebarNav already wired up. This can also be used as a central place to effect all management pages if necessary in the future.

Each management page now needs to thread that prop through to wherever they used to render the old EuiPage* component(s). (How painful this is, is entirely plugin dependent but doesn't seem to bad.)

For the Index Pattern page it required 2 steps:

  1. Pass it into the component
  2. Accept it in the rendering component

For the Index Management page it required 4:

  1. Pass it through to plugin (and update the types)
  2. Pass it through the plugin router
  3. Pass it through the page router
  4. Accept it in the rendering component

Convert a few pages

Converted Management index page, Index Management page, Index Patterns page (though styles might need to be cleaned up a little still)

Screen Shot 2021-04-02 at 15 54 30 PM

Screen Shot 2021-04-02 at 15 52 29 PM

Screen Shot 2021-04-02 at 15 54 13 PM

Need to convert

Here is a list to start tracking the needed conversions for of Management pages and most likely to be turned into a meta ticket in the Kibana repo with assignments based on team ownership:

Logstash

  • Ingest Node Pipelines
  • Logstash Pipelines
  • Beats Central Management

Data

  • Index Management
    • Index page
    • Creation pages
  • Index Lifecycle Policies
  • Snapshot and Restore
  • Rollup Jobs
  • Transforms
  • Cross-Cluster Replication
  • Remote Clusters

Alerts and Insights

  • Rules and Connectors
  • Reporting
  • Machine Learning Jobs
  • Watcher

Security

  • Users
  • Roles
  • API Keys
  • Role Mappings

Kibana

  • Index Patterns
    • Index page
    • View
    • Edit
  • Saved Objects
  • Tags
  • Search Sessions
  • Spaces
  • Advanced Settings (still needs bottom bar and pageTitle adjustments)

Stack

  • License Management
  • 9.0 Upgrade Assistant

Post-follow-up ideas

Allow for each item in the side-nav to add local (nested) items and auto-scroll.

image

cchaos added 3 commits April 2, 2021 15:54
Todo: Fix pageTitle icon and supply bottom bar to layout when upgrading EUI
# Conflicts:
#	src/plugins/kibana_react/public/table_list_view/table_list_view.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants