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

[canvas] New Home Page #102446

Merged
merged 6 commits into from Jun 22, 2021

Conversation

clintandrewhall
Copy link
Contributor

@clintandrewhall clintandrewhall commented Jun 17, 2021

Summary

Fixes #98827

This is a PR containing the new Home Page for Canvas based on the new Analytics Solution page layouts. To test, you can pull the PR, or you can just view the built Storybook! (click the Storybook Preview link from @elasticmachine below).

Screen Shot 2021-06-17 at 12 00 04 AM

Screen Shot 2021-06-17 at 12 00 18 AM

Screen Shot 2021-06-17 at 12 00 08 AM

There's a lot here, but I'll summarize:

Highlights

  • you can test the entire home page in Storybook...!
    • The only exception are services (like create from template) that aren't in the Canvas services architecture.
  • I applied our new best-practice of using service and redux hooks in a component which consumes a functional component for ease of testing.
  • Both the "My Workpad" and "Templates" tabs are loaded with React.lazy
    • This should lower our bundle size.

More

  • Killed a ton of old code.
    • Some of the code was copied over from older components, but all of it was simplified.
  • Removed the Canvas workpad modal in favor of breadcrumb navigation.
  • Wrote new hooks for all services-based interactions.
  • Switched to in-memory tables, (which killed a lot of old, custom code that didn't apply any longer)
  • Wrote Storybook examples for everything.

For Follow-up

  • Check with Eui regarding the space beneath the tab bar in EuiPageLayout.
  • Convert Canvas to use our Presentation Service Abstractions.
    • A lot of the Storybook code would be simpler if we had that.
  • Should we put the hooks somewhere common?

@clintandrewhall clintandrewhall added review WIP Work in progress Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:medium Medium Level of Effort v8.0.0 release_note:skip Skip the PR/issue when compiling release notes impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Feature:Canvas v7.14.0 labels Jun 17, 2021
@clintandrewhall clintandrewhall changed the title [canvas] Refactor Storybook from bespoke to standard configuration [canvas] New Home Page Jun 17, 2021
@clintandrewhall
Copy link
Contributor Author

@elasticmachine merge upstream

@clintandrewhall clintandrewhall requested a review from a team June 17, 2021 05:08
@clintandrewhall
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@clintandrewhall clintandrewhall force-pushed the canvas/page_layouts branch 3 times, most recently from feea01c to e8fd903 Compare June 18, 2021 06:44
@clintandrewhall clintandrewhall removed the WIP Work in progress label Jun 18, 2021
@clintandrewhall clintandrewhall marked this pull request as ready for review June 18, 2021 13:53
@clintandrewhall clintandrewhall requested a review from a team as a code owner June 18, 2021 13:53
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

getUploadFailureErrorMessage: () =>
i18n.translate('xpack.canvas.error.workpadLoader.uploadFailureErrorMessage', {
defaultMessage: `Couldn't upload workpad`,
WorkpadDropzone: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move to component

@clintandrewhall
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
canvas 1090 1115 +25

Async chunks

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

id before after diff
canvas 1.3MB 1.3MB +10.4KB

Page load bundle

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

id before after diff
canvas 531.2KB 520.9KB -10.3KB
Unknown metric groups

async chunk count

id before after diff
canvas 8 10 +2

History

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

Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

Approving. From my understanding, the spacing between the tabs will be fixed by @cchaos in a separate EUI PR.

Copy link
Contributor

@crob611 crob611 left a comment

Choose a reason for hiding this comment

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

Looks great to me.

I like leaving the hooks in the component folder for now, but we will need to consider where to put our "common" hooks that may be used across multiple components in the future. I did the same thing for the hooks used by a lot of the routes.

@clintandrewhall
Copy link
Contributor Author

@crob611 My thoughts, as well. I think when we move to the "feature folder" structure, this will be a lot easier.

@clintandrewhall clintandrewhall added the auto-backport Deprecated: Automatically backport this PR after it's merged label Jun 22, 2021
@clintandrewhall clintandrewhall merged commit 2b0f125 into elastic:master Jun 22, 2021
@clintandrewhall clintandrewhall deleted the canvas/page_layouts branch June 22, 2021 19:11
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Jun 22, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 22, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Clint Andrew Hall <clint.hall@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated: Automatically backport this PR after it's merged Feature:Canvas impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:skip Skip the PR/issue when compiling release notes review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[META] Analytics Solution fixes needed for page layouts
5 participants