Skip to content

refactor MediaUploadModalWrapper to a functional component. - #81652

Closed
aadil42 wants to merge 4 commits into
WordPress:trunkfrom
aadil42:issue/22890/refactor/MediaUploadModalWrapper
Closed

aadil42 wants to merge 4 commits into
WordPress:trunkfrom
aadil42:issue/22890/refactor/MediaUploadModalWrapper

Conversation

@aadil42

@aadil42 aadil42 commented Aug 14, 2026

Copy link
Copy Markdown

What?

Refactors MediaUploadModalWrapper in packages/editor/src/hooks/media-upload.js from a class component to a function component, part of the ongoing class → function migration #22890.

Why?

Refactors as part of the broader class → component migration tracked in #22890.

How?

  • Converts state (isOpen) from this.state to useState
  • Drops the constructor and bound methods (openModal, closeModal) in favor of plain functions
  • render() becomes a direct return, no lifecycle methods needed since there were none to begin with
  • Public API (allowedTypes, multiple, value, onSelect, title, modalClass, render, onClose) is unchanged
  • Adds test coverage for the render-prop trigger, opening the modal, selecting media, and closing without selecting — none previously existed

Testing Instructions

  1. Set window.__experimentalDataViewsMediaModal = true in the browser console (or hardcode the flag temporarily) and reload the editor.
  2. Open a post, click the inserter ("+"), add an Image block.
  3. In the empty Image block, click "Media Library" — the modal should open.
  4. Select an image — modal closes, image is inserted into the block.
  5. Add another Image block, open the modal again, and close it without selecting (X / overlay) — modal closes, block stays empty.
  6. Set the flag to false, reload, and repeat step 2–3 to confirm the stable MediaUpload component still loads normally.

The component previously had no tests. This PR adds coverage for the render-prop trigger, opening the modal, selecting media (calls onSelect and closes), and closing without selecting (calls onClose).

you can run unit test as shown below:

npx jest packages/editor/src/hooks/test/media-upload.js --config test/unit/jest.config.js

Testing Instructions for Keyboard

Use of AI Tools

Assisted by Claude.

@github-actions github-actions Bot added the [Package] Editor /packages/editor label Aug 14, 2026
@github-actions

github-actions Bot commented Aug 14, 2026 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: aadil42 <aadil42@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions github-actions Bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Aug 14, 2026
@github-actions

Copy link
Copy Markdown

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @aadil42! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@Mamaduka Mamaduka added the [Type] Code Quality Issues or PRs that relate to code quality label Aug 18, 2026
@Mamaduka

Copy link
Copy Markdown
Member

Thanks, @aadil42!

Looking at the file history, it seems that the use of a class component is intentional (#73225). cc @tellthemachines

@aadil42

aadil42 commented Aug 18, 2026

Copy link
Copy Markdown
Author

@Mamaduka, Sorry for the late reply. Yeah looking at the file history the class component is intentional here. Some plugins (AMP plugin) are using MediaUpload thinking it to be a class component.

But the last change to the file added deprecated warning

image

So my guess is that after 7.2 version we should refactor it to be a class component.

Should we keep this PR open then so we can merge it later?

@Mamaduka

Copy link
Copy Markdown
Member

It will need further testing. I doubt this will be removed in the recent release.

Let's close this PR so we don't merge refactoring sooner than needed and break consumer code.

Thanks again, @aadil42!

@aadil42 aadil42 closed this Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Package] Editor /packages/editor [Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants