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.

Sign up for GitHub

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

Jump to bottom

[Canvas] Fixes embeddable out of sync in fullscreen mode #109945

Merged
merged 2 commits into from
Aug 27, 2021
Merged

[Canvas] Fixes embeddable out of sync in fullscreen mode #109945

merged 2 commits into from
Aug 27, 2021

Conversation

cqliu1
Copy link
Contributor

@cqliu1 cqliu1 commented Aug 24, 2021 •
edited
Loading

Summary

Closes #108807.

After interacting with an embeddable element in a Canvas workpad, switching to fullscreen mode or view mode would render an old version of the embeddable without all of the updates. This was due to a caching issue with resolved args in Canvas.

To fix the issue, this PR clears out the cached version of the embeddable from the resolved args in transient state on destroy, forcing the embeddable to re-evaluate the expression when switching from edit mode to view/fullscreen mode.

For maintainers

@cqliu1 cqliu1 added bug Fixes for quality problems that affect the customer experience review 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.15.0 v7.16.0 v7.14.1 labels Aug 24, 2021
@cqliu1 cqliu1 requested a review from a team as a code owner August 24, 2021 20:21
@elasticmachine
Copy link
Contributor

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

crob611
crob611 approved these changes Aug 25, 2021
View reviewed changes
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.

👍

@cqliu1 cqliu1 force-pushed the fix/embeddable-fullscreen-mode branch from 4878079 to dfc6917 Compare August 25, 2021 20:41
@cqliu1 cqliu1 force-pushed the fix/embeddable-fullscreen-mode branch from dfc6917 to 2a7e846 Compare August 25, 2021 20:45
@cqliu1 cqliu1 added the auto-backport Deprecated - use backport:version if exact versions are needed label Aug 27, 2021
@cqliu1
Copy link
Contributor Author

cqliu1 commented Aug 27, 2021

@elasticmachine merge upstream

@cqliu1 cqliu1 enabled auto-merge (squash) August 27, 2021 04:32
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

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.6MB 1.6MB +97.0B

History

  • 💚 Build #148348 succeeded 487807904fabe2401834e274a9b7b050bd3f7913

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

@cqliu1 cqliu1 merged commit 2e4e0fc into elastic:master Aug 27, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Aug 27, 2021
@cqliu1 @kibanamachine
Clears resolved arg on embeddable destroy (elastic#109945)
e5fa5b9 
Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request Aug 27, 2021
Merged
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Aug 27, 2021
@cqliu1 @kibanamachine
Clears resolved arg on embeddable destroy (elastic#109945)
accc77f 
Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request Aug 27, 2021
Merged
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Aug 27, 2021
@cqliu1 @kibanamachine
Clears resolved arg on embeddable destroy (elastic#109945)
5ee6c2b 
Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request Aug 27, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.15
✅ 7.x
✅ 7.14

The backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Aug 27, 2021
@kibanamachine @cqliu1
Clears resolved arg on embeddable destroy (#109945) (#110393)
aacdc90 
Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Catherine Liu <[email protected]>
kibanamachine added a commit that referenced this pull request Aug 27, 2021
@kibanamachine @cqliu1
Clears resolved arg on embeddable destroy (#109945) (#110391)
785ea50 
Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Catherine Liu <[email protected]>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 31, 2021
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@cqliu1 cqliu1 deleted the fix/embeddable-fullscreen-mode branch August 31, 2021 15:02
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 31, 2021
kibanamachine added a commit that referenced this pull request Aug 31, 2021
@kibanamachine @cqliu1
Clears resolved arg on embeddable destroy (#109945) (#110392)
5d3b827 
Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Catherine Liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crob611 crob611 crob611 approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed bug Fixes for quality problems that affect the customer experience 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.2 v7.15.0 v7.16.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Canvas] Embeddable changes don't persist in fullscreen mode
5 participants
@cqliu1 @elasticmachine @kibanamachine @crob611 @rashmivkulkarni