[Embeddables Rebuild] Clean up RenderCompleteDispatcher and related attributes from all embeddables
#179376
Labels
blocked
Feature:Embeddables
Relating to the Embeddable system
impact:critical
This issue should be addressed immediately due to a critical level of impact on the product.
loe:medium
Medium Level of Effort
project:embeddableRebuild
Team:Presentation
Presentation Team for Dashboard, Input Controls, and Canvas
Comments
Heenawter
added
Team:Presentation
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
Heenawter
changed the title
RenderCompleteDispatcher and related props from all embeddablesRenderCompleteDispatcher and related attributes from all embeddables
|
Marking this as blocked, since it is too high risk to do this until all embeddables have been converted. |
This was referenced Apr 23, 2024
|
@Heenawter Regarding I don't know more details about rest attributes, just wanted to share about data-shared-item. I must admit that it is not straightforward what this attribute is doing. But I guess part of the current issue you created, is to figure out what all these attributes are used for and possibly refactor. |
mgiota
added a commit
that referenced
this issue
Apr 26, 2024
Fixes #181389 It turns out that a [data-shared-item is needed](https://github.com/elastic/kibana/pull/169929/files#r1373148068), otherwise reporting doesn't work properly. This PR is adding the required `data-shared-item` to the presentation panel component, and fixes the reporting screenshot issue. **UPDATE**: Adding `data-shared-item` to the presentation panel caused some test failures. The approach we followed for now, was to add this attribute to each migrated embeddable, the `image` and `swim lane` embeddables. As part of this #179376, Kibana presentation team will investigate further the proper use of data-* attributes ## Before the fix <img width="600" alt="Screenshot 2024-04-23 at 10 41 59" src="https://github.com/elastic/kibana/assets/2852703/cee076a1-b989-4d5f-8462-4021ce9e5e4d"> <img width="600" alt="Screenshot 2024-04-23 at 10 41 27" src="https://github.com/elastic/kibana/assets/2852703/83677ad1-b1d2-4915-a747-9afe5a1d447a"> ## ✔️ Acceptance criteria - No timeout error should appear in the generated PDF reports ## After the fix <img width="600" alt="Screenshot 2024-04-23 at 11 02 32" src="https://github.com/elastic/kibana/assets/2852703/e0452e32-8c1e-4075-b9c9-b1225f9bd852"> <img width="600" alt="Screenshot 2024-04-23 at 11 19 54" src="https://github.com/elastic/kibana/assets/2852703/08284774-a4ff-47b0-b496-3570416f0e57"> <img width="600" alt="Screenshot 2024-04-23 at 11 07 43" src="https://github.com/elastic/kibana/assets/2852703/95e9adae-cd07-42fe-9dea-cd22b9711155"> cc @Heenawter @darnautov Can you check the swim lane embeddable with real data and see if reporting works properly with the change I added? --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Hannah Mudge <hannah.wright@elastic.co>
mgiota
added a commit
to mgiota/kibana
that referenced
this issue
Apr 26, 2024
…81392) Fixes elastic#181389 It turns out that a [data-shared-item is needed](https://github.com/elastic/kibana/pull/169929/files#r1373148068), otherwise reporting doesn't work properly. This PR is adding the required `data-shared-item` to the presentation panel component, and fixes the reporting screenshot issue. **UPDATE**: Adding `data-shared-item` to the presentation panel caused some test failures. The approach we followed for now, was to add this attribute to each migrated embeddable, the `image` and `swim lane` embeddables. As part of this elastic#179376, Kibana presentation team will investigate further the proper use of data-* attributes <img width="600" alt="Screenshot 2024-04-23 at 10 41 59" src="https://github.com/elastic/kibana/assets/2852703/cee076a1-b989-4d5f-8462-4021ce9e5e4d"> <img width="600" alt="Screenshot 2024-04-23 at 10 41 27" src="https://github.com/elastic/kibana/assets/2852703/83677ad1-b1d2-4915-a747-9afe5a1d447a"> - No timeout error should appear in the generated PDF reports <img width="600" alt="Screenshot 2024-04-23 at 11 02 32" src="https://github.com/elastic/kibana/assets/2852703/e0452e32-8c1e-4075-b9c9-b1225f9bd852"> <img width="600" alt="Screenshot 2024-04-23 at 11 19 54" src="https://github.com/elastic/kibana/assets/2852703/08284774-a4ff-47b0-b496-3570416f0e57"> <img width="600" alt="Screenshot 2024-04-23 at 11 07 43" src="https://github.com/elastic/kibana/assets/2852703/95e9adae-cd07-42fe-9dea-cd22b9711155"> cc @Heenawter @darnautov Can you check the swim lane embeddable with real data and see if reporting works properly with the change I added? --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Hannah Mudge <hannah.wright@elastic.co> (cherry picked from commit f1e02f6)
mgiota
added a commit
that referenced
this issue
Apr 29, 2024
…81392) (#181913) # Backport This will backport the following commits from `main` to `8.14`: - [[Embeddable rebuild] Fix kibana reporting screenshot issue (#181392)](#181392) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Panagiota Mitsopoulou","email":"panagiota.mitsopoulou@elastic.co"},"sourceCommit":{"committedDate":"2024-04-26T13:31:24Z","message":"[Embeddable rebuild] Fix kibana reporting screenshot issue (#181392)\n\nFixes https://github.com/elastic/kibana/issues/181389\r\n\r\nIt turns out that a [data-shared-item is\r\nneeded](https://github.com/elastic/kibana/pull/169929/files#r1373148068),\r\notherwise reporting doesn't work properly. This PR is adding the\r\nrequired `data-shared-item` to the presentation panel component, and\r\nfixes the reporting screenshot issue.\r\n\r\n**UPDATE**: Adding `data-shared-item` to the presentation panel caused\r\nsome test failures. The approach we followed for now, was to add this\r\nattribute to each migrated embeddable, the `image` and `swim lane`\r\nembeddables. As part of this\r\nhttps://github.com//issues/179376, Kibana presentation\r\nteam will investigate further the proper use of data-* attributes\r\n\r\n## Before the fix\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 10 41 59\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/cee076a1-b989-4d5f-8462-4021ce9e5e4d\">\r\n\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 10 41 27\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/83677ad1-b1d2-4915-a747-9afe5a1d447a\">\r\n\r\n\r\n## ✔️ Acceptance criteria\r\n- No timeout error should appear in the generated PDF reports\r\n\r\n## After the fix\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 11 02 32\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/e0452e32-8c1e-4075-b9c9-b1225f9bd852\">\r\n\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 11 19 54\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/08284774-a4ff-47b0-b496-3570416f0e57\">\r\n\r\n\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 11 07 43\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/95e9adae-cd07-42fe-9dea-cd22b9711155\">\r\n\r\ncc @Heenawter \r\n@darnautov Can you check the swim lane embeddable with real data and see\r\nif reporting works properly with the change I added?\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Hannah Mudge <hannah.wright@elastic.co>","sha":"f1e02f642247620b90770591297efccb957eee1e","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","release_note:skip","Feature:Embeddables","Team:obs-ux-management","project:embeddableRebuild","v8.14.0","v8.15.0"],"number":181392,"url":"https://github.com/elastic/kibana/pull/181392","mergeCommit":{"message":"[Embeddable rebuild] Fix kibana reporting screenshot issue (#181392)\n\nFixes https://github.com/elastic/kibana/issues/181389\r\n\r\nIt turns out that a [data-shared-item is\r\nneeded](https://github.com/elastic/kibana/pull/169929/files#r1373148068),\r\notherwise reporting doesn't work properly. This PR is adding the\r\nrequired `data-shared-item` to the presentation panel component, and\r\nfixes the reporting screenshot issue.\r\n\r\n**UPDATE**: Adding `data-shared-item` to the presentation panel caused\r\nsome test failures. The approach we followed for now, was to add this\r\nattribute to each migrated embeddable, the `image` and `swim lane`\r\nembeddables. As part of this\r\nhttps://github.com//issues/179376, Kibana presentation\r\nteam will investigate further the proper use of data-* attributes\r\n\r\n## Before the fix\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 10 41 59\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/cee076a1-b989-4d5f-8462-4021ce9e5e4d\">\r\n\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 10 41 27\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/83677ad1-b1d2-4915-a747-9afe5a1d447a\">\r\n\r\n\r\n## ✔️ Acceptance criteria\r\n- No timeout error should appear in the generated PDF reports\r\n\r\n## After the fix\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 11 02 32\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/e0452e32-8c1e-4075-b9c9-b1225f9bd852\">\r\n\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 11 19 54\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/08284774-a4ff-47b0-b496-3570416f0e57\">\r\n\r\n\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 11 07 43\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/95e9adae-cd07-42fe-9dea-cd22b9711155\">\r\n\r\ncc @Heenawter \r\n@darnautov Can you check the swim lane embeddable with real data and see\r\nif reporting works properly with the change I added?\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Hannah Mudge <hannah.wright@elastic.co>","sha":"f1e02f642247620b90770591297efccb957eee1e"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","labelRegex":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/181392","number":181392,"mergeCommit":{"message":"[Embeddable rebuild] Fix kibana reporting screenshot issue (#181392)\n\nFixes https://github.com/elastic/kibana/issues/181389\r\n\r\nIt turns out that a [data-shared-item is\r\nneeded](https://github.com/elastic/kibana/pull/169929/files#r1373148068),\r\notherwise reporting doesn't work properly. This PR is adding the\r\nrequired `data-shared-item` to the presentation panel component, and\r\nfixes the reporting screenshot issue.\r\n\r\n**UPDATE**: Adding `data-shared-item` to the presentation panel caused\r\nsome test failures. The approach we followed for now, was to add this\r\nattribute to each migrated embeddable, the `image` and `swim lane`\r\nembeddables. As part of this\r\nhttps://github.com//issues/179376, Kibana presentation\r\nteam will investigate further the proper use of data-* attributes\r\n\r\n## Before the fix\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 10 41 59\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/cee076a1-b989-4d5f-8462-4021ce9e5e4d\">\r\n\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 10 41 27\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/83677ad1-b1d2-4915-a747-9afe5a1d447a\">\r\n\r\n\r\n## ✔️ Acceptance criteria\r\n- No timeout error should appear in the generated PDF reports\r\n\r\n## After the fix\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 11 02 32\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/e0452e32-8c1e-4075-b9c9-b1225f9bd852\">\r\n\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 11 19 54\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/08284774-a4ff-47b0-b496-3570416f0e57\">\r\n\r\n\r\n<img width=\"600\" alt=\"Screenshot 2024-04-23 at 11 07 43\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2852703/95e9adae-cd07-42fe-9dea-cd22b9711155\">\r\n\r\ncc @Heenawter \r\n@darnautov Can you check the swim lane embeddable with real data and see\r\nif reporting works properly with the change I added?\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Hannah Mudge <hannah.wright@elastic.co>","sha":"f1e02f642247620b90770591297efccb957eee1e"}}]}] BACKPORT--> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
4 tasks
|
Visualize embeddables need to dispatch a Not sure how many other embeddable types need to do this. If it's a wider issue we should also make sure they're dispatching this event on the framework level instead of having to make each embeddable type do it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the feature:
Currently, for all legacy embeddables, we are ending up with duplicated
data-render-completeattributes - (1) from the embeddable itself (due to the use ofRenderCompleteDispatcherand/or setting the props manually somewhere in the embeddable code) and (2) from the wrappingPresentationPanelcomponent:Once every embeddable has been converted to the new React system,
PresentationPanelshould be entirely responsible for handling thedata-render-complete,data-rendering-count,data-loading,data-rendering-error, anddata-errorattributes. As part of this work, we will need to check that no embeddables are continuing to useRenderCompleteDispatcherand/or setting these attributes manually - and, we can hopefully removeRenderCompleteDispatcherentirely once we are done.Open questions:
I'm not entirely sure what we want to do with the other
data-*attributes - for example,data-title,data-shared-item,data-shared-items-container, ... etc. We should dig into exactly what these attributes are used for (if they are used at all) and try to centralize where these things are being set. PerhapsPresentationPanel, perhaps somewhere else.The text was updated successfully, but these errors were encountered: