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

[Lens] Add the bee #136132

Merged
merged 6 commits into from
Jul 18, 2022
Merged

[Lens] Add the bee #136132

merged 6 commits into from
Jul 18, 2022

Conversation

flash1293
Copy link
Contributor

This adds the bee

@flash1293 flash1293 added Team:Visualizations Visualization editors, elastic-charts and infrastructure release_note:skip Skip the PR/issue when compiling release notes Feature:Lens backport:skip This commit does not require backporting v8.4.0 labels Jul 11, 2022
@flash1293 flash1293 marked this pull request as ready for review July 12, 2022 16:23
@flash1293 flash1293 requested a review from a team as a code owner July 12, 2022 16:23
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

@flash1293 flash1293 marked this pull request as draft July 12, 2022 16:24
@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@flash1293 flash1293 marked this pull request as ready for review July 13, 2022 14:51
Copy link
Contributor

@drewdaemon drewdaemon left a comment

Choose a reason for hiding this comment

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

[redacted]

Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

This is pretty cute.

image

Some comments to further reduce its footprint.

);
}

export function Easteregg(props: { query?: Query }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This loads the code (even though Lazy), even when the easter-egg query isn't triggered. Consider only loading the easter-egg when the query actually matches.

So I'd break here and return null here on the REGEX-check. And only load the Bee lazily after.

Copy link
Contributor Author

@flash1293 flash1293 Jul 14, 2022

Choose a reason for hiding this comment

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

This is what the code does - the Bees component does nothing but checking the query in a safe way. Only if the amount is larger than one and a bee will be shown, a Bee component is rendered which will lazy-load the bee code (including the image). The Easteregg component is providing the error boundary, so an uncaught exception in the bee/bees component won't be visible in the app.

Copy link
Contributor

Choose a reason for hiding this comment

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

oki thx, you're right, I have misunderstood then

@timductive
Copy link
Member

feature request for even more bees please

@flash1293 flash1293 enabled auto-merge (squash) July 18, 2022 07:24
Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

I love this!! LGTM <3

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 930 933 +3

Async chunks

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

id before after diff
lens 1.2MB 1.2MB +1.7KB

Page load bundle

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

id before after diff
lens 34.0KB 34.0KB +1.0B
Unknown metric groups

async chunk count

id before after diff
lens 13 14 +1

ESLint disabled line counts

id before after diff
lens 28 29 +1

miscellaneous assets size

id before after diff
lens 247.3KB 310.2KB +62.9KB

Total ESLint disabled count

id before after diff
lens 30 31 +1

History

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

@flash1293 flash1293 merged commit a5738e9 into elastic:main Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants