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

Media: Remove media frame on close #12217

Merged
merged 1 commit into from
Sep 12, 2022
Merged

Media: Remove media frame on close #12217

merged 1 commit into from
Sep 12, 2022

Conversation

spacedmonkey
Copy link
Contributor

@spacedmonkey spacedmonkey commented Sep 5, 2022

Context

Summary

While working on #11941. I discovered that wp media instance when the wp media instance is closed. This instance may use memory in the browser and requires resetting. Just use remove method.

See

https://github.com/WordPress/gutenberg/blob/67a652439c3d2f7dd8623d24b0bd12b5aff053f5/packages/media-utils/src/components/media-upload/index.js#L342-L344

Relevant Technical Choices

To-do

User-facing changes

Testing Instructions

  • This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

Reviews

Does this PR have a security-related impact?

Does this PR change what data or activity we track or use?

Does this PR have a legal-related impact?

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Fixes #

@googleforcreators-bot
Copy link
Collaborator

Plugin builds for 59a21e5 are ready 🛎️!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2022

Size Change: -56 B (0%)

Total Size: 2.68 MB

ℹ️ View Unchanged
Filename Size Change
assets/css/carousel-view-rtl.css 702 B 0 B
assets/css/carousel-view.css 701 B 0 B
assets/css/web-stories-block-rtl.css 4.52 kB 0 B
assets/css/web-stories-block.css 4.56 kB 0 B
assets/css/web-stories-embed-rtl.css 318 B 0 B
assets/css/web-stories-embed.css 317 B 0 B
assets/css/web-stories-list-styles-rtl.css 2.36 kB 0 B
assets/css/web-stories-list-styles.css 2.39 kB 0 B
assets/css/web-stories-theme-style-twentyeleven-rtl.css 102 B 0 B
assets/css/web-stories-theme-style-twentyeleven.css 102 B 0 B
assets/css/web-stories-theme-style-twentyfifteen-rtl.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfifteen.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfourteen-rtl.css 287 B 0 B
assets/css/web-stories-theme-style-twentyfourteen.css 287 B 0 B
assets/css/web-stories-theme-style-twentyseventeen-rtl.css 288 B 0 B
assets/css/web-stories-theme-style-twentyseventeen.css 288 B 0 B
assets/css/web-stories-theme-style-twentysixteen-rtl.css 224 B 0 B
assets/css/web-stories-theme-style-twentysixteen.css 224 B 0 B
assets/css/web-stories-theme-style-twentyten-rtl.css 143 B 0 B
assets/css/web-stories-theme-style-twentyten.css 143 B 0 B
assets/css/web-stories-theme-style-twentytwelve-rtl.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwelve.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwenty-rtl.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwenty.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwentyone-rtl.css 326 B 0 B
assets/css/web-stories-theme-style-twentytwentyone.css 326 B 0 B
assets/css/web-stories-widget-rtl.css 482 B 0 B
assets/css/web-stories-widget.css 482 B 0 B
assets/css/wp-dashboard-rtl.css 657 B 0 B
assets/css/wp-dashboard.css 659 B 0 B
assets/css/wp-story-editor-rtl.css 737 B 0 B
assets/css/wp-story-editor.css 738 B 0 B
assets/js/1814.js 7.45 kB 0 B
assets/js/4317.js 37.1 kB 0 B
assets/js/4422.js 49.3 kB 0 B
assets/js/6527.js 222 kB 0 B
assets/js/7903.js 5.82 kB 0 B
assets/js/9589.js 1.14 MB 0 B
assets/js/carousel-view.js 3.41 kB 0 B
assets/js/chunk-colorthief.js 2.64 kB 0 B
assets/js/chunk-ffmpeg.js 5.64 kB 0 B
assets/js/chunk-focus-visible.js 1.01 kB 0 B
assets/js/chunk-getStoryMarkup.js 5.79 kB 0 B
assets/js/chunk-html-to-image.js 4.55 kB 0 B
assets/js/chunk-opentype.js 96 B 0 B
assets/js/chunk-react-calendar.js 12.4 kB 0 B
assets/js/chunk-react-color.js 44.3 kB 0 B
assets/js/chunk-web-animations-js.js 14.6 kB 0 B
assets/js/chunk-web-stories-template-0-metaData.js 545 B 0 B
assets/js/chunk-web-stories-template-0.js 11.4 kB 0 B
assets/js/chunk-web-stories-template-1-metaData.js 539 B 0 B
assets/js/chunk-web-stories-template-1.js 9.61 kB 0 B
assets/js/chunk-web-stories-template-10-metaData.js 533 B 0 B
assets/js/chunk-web-stories-template-10.js 7.38 kB 0 B
assets/js/chunk-web-stories-template-11-metaData.js 539 B 0 B
assets/js/chunk-web-stories-template-11.js 9.08 kB 0 B
assets/js/chunk-web-stories-template-12-metaData.js 497 B 0 B
assets/js/chunk-web-stories-template-12.js 9.69 kB 0 B
assets/js/chunk-web-stories-template-13-metaData.js 525 B 0 B
assets/js/chunk-web-stories-template-13.js 7.43 kB 0 B
assets/js/chunk-web-stories-template-14-metaData.js 582 B 0 B
assets/js/chunk-web-stories-template-14.js 7.39 kB 0 B
assets/js/chunk-web-stories-template-15-metaData.js 544 B 0 B
assets/js/chunk-web-stories-template-15.js 9.02 kB 0 B
assets/js/chunk-web-stories-template-16-metaData.js 588 B 0 B
assets/js/chunk-web-stories-template-16.js 10.9 kB 0 B
assets/js/chunk-web-stories-template-17-metaData.js 539 B 0 B
assets/js/chunk-web-stories-template-17.js 9.2 kB 0 B
assets/js/chunk-web-stories-template-18-metaData.js 585 B 0 B
assets/js/chunk-web-stories-template-18.js 9.91 kB 0 B
assets/js/chunk-web-stories-template-19-metaData.js 501 B 0 B
assets/js/chunk-web-stories-template-19.js 10.8 kB 0 B
assets/js/chunk-web-stories-template-2-metaData.js 586 B 0 B
assets/js/chunk-web-stories-template-2.js 9.33 kB 0 B
assets/js/chunk-web-stories-template-20-metaData.js 548 B 0 B
assets/js/chunk-web-stories-template-20.js 9.02 kB 0 B
assets/js/chunk-web-stories-template-21-metaData.js 535 B 0 B
assets/js/chunk-web-stories-template-21.js 9.86 kB 0 B
assets/js/chunk-web-stories-template-22-metaData.js 525 B 0 B
assets/js/chunk-web-stories-template-22.js 7.83 kB 0 B
assets/js/chunk-web-stories-template-23-metaData.js 605 B 0 B
assets/js/chunk-web-stories-template-23.js 7.48 kB 0 B
assets/js/chunk-web-stories-template-24-metaData.js 516 B 0 B
assets/js/chunk-web-stories-template-24.js 11.7 kB 0 B
assets/js/chunk-web-stories-template-25-metaData.js 544 B 0 B
assets/js/chunk-web-stories-template-25.js 7.08 kB 0 B
assets/js/chunk-web-stories-template-26-metaData.js 601 B 0 B
assets/js/chunk-web-stories-template-26.js 7.27 kB 0 B
assets/js/chunk-web-stories-template-27-metaData.js 542 B 0 B
assets/js/chunk-web-stories-template-27.js 7.82 kB 0 B
assets/js/chunk-web-stories-template-28-metaData.js 532 B 0 B
assets/js/chunk-web-stories-template-28.js 9.08 kB 0 B
assets/js/chunk-web-stories-template-29-metaData.js 562 B 0 B
assets/js/chunk-web-stories-template-29.js 9.27 kB 0 B
assets/js/chunk-web-stories-template-3-metaData.js 539 B 0 B
assets/js/chunk-web-stories-template-3.js 8.44 kB 0 B
assets/js/chunk-web-stories-template-30-metaData.js 576 B 0 B
assets/js/chunk-web-stories-template-30.js 7.91 kB 0 B
assets/js/chunk-web-stories-template-31-metaData.js 502 B 0 B
assets/js/chunk-web-stories-template-31.js 10.3 kB 0 B
assets/js/chunk-web-stories-template-32-metaData.js 552 B 0 B
assets/js/chunk-web-stories-template-32.js 13.3 kB 0 B
assets/js/chunk-web-stories-template-33-metaData.js 492 B 0 B
assets/js/chunk-web-stories-template-33.js 9.07 kB 0 B
assets/js/chunk-web-stories-template-34-metaData.js 571 B 0 B
assets/js/chunk-web-stories-template-34.js 7.58 kB 0 B
assets/js/chunk-web-stories-template-35-metaData.js 566 B 0 B
assets/js/chunk-web-stories-template-35.js 8.91 kB 0 B
assets/js/chunk-web-stories-template-36-metaData.js 577 B 0 B
assets/js/chunk-web-stories-template-36.js 12.7 kB 0 B
assets/js/chunk-web-stories-template-37-metaData.js 528 B 0 B
assets/js/chunk-web-stories-template-37.js 6.71 kB 0 B
assets/js/chunk-web-stories-template-38-metaData.js 572 B 0 B
assets/js/chunk-web-stories-template-38.js 7.95 kB 0 B
assets/js/chunk-web-stories-template-39-metaData.js 588 B 0 B
assets/js/chunk-web-stories-template-39.js 8.09 kB 0 B
assets/js/chunk-web-stories-template-4-metaData.js 564 B 0 B
assets/js/chunk-web-stories-template-4.js 12.7 kB 0 B
assets/js/chunk-web-stories-template-40-metaData.js 557 B 0 B
assets/js/chunk-web-stories-template-40.js 10.2 kB 0 B
assets/js/chunk-web-stories-template-41-metaData.js 572 B 0 B
assets/js/chunk-web-stories-template-41.js 7.77 kB 0 B
assets/js/chunk-web-stories-template-42-metaData.js 521 B 0 B
assets/js/chunk-web-stories-template-42.js 7 kB 0 B
assets/js/chunk-web-stories-template-43-metaData.js 557 B 0 B
assets/js/chunk-web-stories-template-43.js 8.78 kB 0 B
assets/js/chunk-web-stories-template-44-metaData.js 584 B 0 B
assets/js/chunk-web-stories-template-44.js 11.1 kB 0 B
assets/js/chunk-web-stories-template-45-metaData.js 565 B 0 B
assets/js/chunk-web-stories-template-45.js 7.51 kB 0 B
assets/js/chunk-web-stories-template-46-metaData.js 531 B 0 B
assets/js/chunk-web-stories-template-46.js 5.22 kB 0 B
assets/js/chunk-web-stories-template-47-metaData.js 592 B 0 B
assets/js/chunk-web-stories-template-47.js 9.42 kB 0 B
assets/js/chunk-web-stories-template-48-metaData.js 555 B 0 B
assets/js/chunk-web-stories-template-48.js 9.09 kB 0 B
assets/js/chunk-web-stories-template-49-metaData.js 518 B 0 B
assets/js/chunk-web-stories-template-49.js 9.7 kB 0 B
assets/js/chunk-web-stories-template-5-metaData.js 556 B 0 B
assets/js/chunk-web-stories-template-5.js 9.96 kB 0 B
assets/js/chunk-web-stories-template-50-metaData.js 503 B 0 B
assets/js/chunk-web-stories-template-50.js 9.16 kB 0 B
assets/js/chunk-web-stories-template-51-metaData.js 526 B 0 B
assets/js/chunk-web-stories-template-51.js 10.4 kB 0 B
assets/js/chunk-web-stories-template-52-metaData.js 602 B 0 B
assets/js/chunk-web-stories-template-52.js 10.4 kB 0 B
assets/js/chunk-web-stories-template-53-metaData.js 553 B 0 B
assets/js/chunk-web-stories-template-53.js 5.79 kB 0 B
assets/js/chunk-web-stories-template-54-metaData.js 547 B 0 B
assets/js/chunk-web-stories-template-54.js 7.66 kB 0 B
assets/js/chunk-web-stories-template-55-metaData.js 573 B 0 B
assets/js/chunk-web-stories-template-55.js 7.13 kB 0 B
assets/js/chunk-web-stories-template-56-metaData.js 542 B 0 B
assets/js/chunk-web-stories-template-56.js 9.89 kB 0 B
assets/js/chunk-web-stories-template-57-metaData.js 527 B 0 B
assets/js/chunk-web-stories-template-57.js 14.9 kB 0 B
assets/js/chunk-web-stories-template-58-metaData.js 555 B 0 B
assets/js/chunk-web-stories-template-58.js 5.75 kB 0 B
assets/js/chunk-web-stories-template-59-metaData.js 590 B 0 B
assets/js/chunk-web-stories-template-59.js 8.97 kB 0 B
assets/js/chunk-web-stories-template-6-metaData.js 569 B 0 B
assets/js/chunk-web-stories-template-6.js 7.06 kB 0 B
assets/js/chunk-web-stories-template-60-metaData.js 510 B 0 B
assets/js/chunk-web-stories-template-60.js 9.64 kB 0 B
assets/js/chunk-web-stories-template-7-metaData.js 569 B 0 B
assets/js/chunk-web-stories-template-7.js 7.46 kB 0 B
assets/js/chunk-web-stories-template-8-metaData.js 569 B 0 B
assets/js/chunk-web-stories-template-8.js 8.94 kB 0 B
assets/js/chunk-web-stories-template-9-metaData.js 581 B 0 B
assets/js/chunk-web-stories-template-9.js 8.48 kB 0 B
assets/js/chunk-web-stories-templates.js 443 B 0 B
assets/js/chunk-web-stories-textset-0.js 5.08 kB 0 B
assets/js/chunk-web-stories-textset-1.js 6.67 kB 0 B
assets/js/chunk-web-stories-textset-2.js 7.67 kB 0 B
assets/js/chunk-web-stories-textset-3.js 15.1 kB 0 B
assets/js/chunk-web-stories-textset-4.js 4.16 kB 0 B
assets/js/chunk-web-stories-textset-5.js 5.49 kB 0 B
assets/js/chunk-web-stories-textset-6.js 5.3 kB 0 B
assets/js/chunk-web-stories-textset-7.js 10.2 kB 0 B
assets/js/generateBlurhash.worker.worker.js 1.1 kB 0 B
assets/js/imgareaselect.js 3.77 kB 0 B
assets/js/lightbox.js 550 B 0 B
assets/js/tinymce-button.js 2.84 kB 0 B
assets/js/web-stories-activation-notice.js 25.4 kB 0 B
assets/js/web-stories-block.js 18 kB 0 B
assets/js/web-stories-embed.js 20 B 0 B
assets/js/web-stories-widget.js 587 B 0 B
assets/js/wp-dashboard.js 72.5 kB 0 B
assets/js/wp-story-editor.js 331 kB -56 B (0%)

compressed-size-action

@swissspidy swissspidy added Type: Code Quality Things that need a refactor, rewrite or just some good old developer ❤️ Group: WordPress Changes related to WordPress or Gutenberg integration Pod: WP labels Sep 5, 2022
@swissspidy swissspidy changed the title Remove frame on close. Media: Remove media frame on close Sep 7, 2022
@swissspidy
Copy link
Collaborator

swissspidy commented Sep 7, 2022

Any recommendations on how to verify this change (testing instructions)? Should there be any changes in the DOM?

@spacedmonkey
Copy link
Contributor Author

Any recommendations on how to verify this change (testing instructions)? Should there be any changes in the DOM?

No dom change here. It is more about reseting state. One way to test this is to open a wp dialog, search for something and close it. Open again.

Discovery this need while looking at this.

@swissspidy
Copy link
Collaborator

One way to test this is to open a wp dialog, search for something and close it. Open again.

I don't see a difference when doing that. What should be different?

@spacedmonkey
Copy link
Contributor Author

I don't see a difference when doing that. What should be different?

In my PR this, is no longer required.

fileFrame.once('content:activate:browse', () => {
// Force-refresh media modal contents every time
// to avoid stale data.
fileFrame.content?.get()?.collection?._requery(true);
fileFrame.content?.get()?.options?.selection?.reset();
});

As removing the the wp.media, means state is reset and no chance of state set in the first instance of using the dialog is not used.

@swissspidy
Copy link
Collaborator

I suppose that explains why there is no difference: there should be none.

@swissspidy swissspidy merged commit 65b2703 into main Sep 12, 2022
@swissspidy swissspidy deleted the remove-frame branch September 12, 2022 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Media Group: WordPress Changes related to WordPress or Gutenberg integration Type: Code Quality Things that need a refactor, rewrite or just some good old developer ❤️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants