Skip to content

Update dropzone context API#11100

Merged
oandregal merged 19 commits into
masterfrom
update/dropzone-context-api
Oct 26, 2018
Merged

Update dropzone context API#11100
oandregal merged 19 commits into
masterfrom
update/dropzone-context-api

Conversation

@oandregal

@oandregal oandregal commented Oct 26, 2018

Copy link
Copy Markdown
Member

The DropZoneProvider is using the React's legacy context API which will be removed in the next major version and was added as a deprecation in StrictMode in 16.6.0.

This PR updates the DropZoneProvider to the stable React's context API. Note that DropZone functionality has remained untouched and works as it used to.

@oandregal oandregal self-assigned this Oct 26, 2018
@oandregal oandregal added [Status] In Progress Tracking issues with work in progress [Type] Breaking Change For PRs that introduce a change that will break existing functionality labels Oct 26, 2018
@oandregal oandregal requested a review from a team October 26, 2018 10:59
@oandregal

Copy link
Copy Markdown
Member Author

One thing we can do is:

  • to maintain the old DropZoneProvider as it is.
  • to create the new one in a new context.js file that DropZone will use and export it as any other component.
  • to load both providers from EditorProvider.

I'm unsure on how best handle name migration. I guess we need to use a different name for the new provider (how about DZProvider?). I wish there was a better way for us to keep using the old DropZoneProvider name after the removal.

@oandregal

Copy link
Copy Markdown
Member Author

As per this slack convo: we don't consider public API the use of DropZoneProvider alone with a consumer other than DropZone. For that reason, this isn't a breaking change because, externally, nothing changes.

@oandregal oandregal added this to the 4.2 milestone Oct 26, 2018
@oandregal oandregal added [Feature] Drag and Drop Drag and drop functionality when working with blocks and removed [Type] Breaking Change For PRs that introduce a change that will break existing functionality [Status] In Progress Tracking issues with work in progress labels Oct 26, 2018
Comment thread packages/components/src/drop-zone/index.js
Comment thread packages/components/src/drop-zone/index.js Outdated
Comment thread packages/components/src/drop-zone/provider.js
Comment thread packages/components/src/drop-zone/provider.js
type: null,
} );

const getDragEventType = ( { dataTransfer } ) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess this could be tested now :)

Comment thread packages/components/src/drop-zone/provider.js Outdated
Comment thread packages/components/src/drop-zone/provider.js Outdated
const { children } = this.props;
return children;
return (
<Provider value={ this.dropZoneCallbacks }>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Although this is the same as before: how about making DropZone a PureComponent? That way, we prevent doing re-renders every time we setState in this component. We already take care of telling dropZones to render individually (on reset, and if the dropZone needs updating.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We do need to rerender the dropzone when its state changes. Whether this state is stored in the dropzone it self or in the provider is not that important is it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

mmm, I'm not sure I follow. Let me clarify what I'm saying:

  • we render the DropZone by explicitly calling its setState method from the DropZoneProvider in the places I mentioned.
  • we also render the DropZone by the normal React mechanism: everytime the DropZoneProvider is rendered its children also are. This is the case I think we may want to prevent by converting DropZone to a PureComponent.

Does it make more sense now?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not really 🤷‍♂️ How do you transform it to a pure component while keeping setState?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh you mean pure by wrapping it in our pure HoC :). In theory, this shouldn't be necessary. The question could be, why do we not wrap all of our components with it?

The response is that it should probably be optimized on a upper level. and this could hurt more than it helps.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've just found out that we don't make PureComponent available through our element package and instead offer the pure HOC. Out of curiosity, why we do that?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pure works for both functional and class component. PureComponent is a base class only.

Comment thread packages/components/src/drop-zone/provider.js

@youknowriad youknowriad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

@oandregal oandregal merged commit a548961 into master Oct 26, 2018
@oandregal oandregal deleted the update/dropzone-context-api branch October 26, 2018 14:23
@mtias mtias added the [Type] Code Quality Issues or PRs that relate to code quality label Oct 30, 2018
adamsilverstein added a commit that referenced this pull request Mar 1, 2026
Links Gutenberg PR #74903 (dimension validation for sideload endpoint)
to its corresponding WordPress Core PR.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
adamsilverstein added a commit that referenced this pull request May 15, 2026
* Add dimension validation to sideload endpoint

Validates uploaded image dimensions against expected size constraints
in the wp/v2/media/<id>/sideload endpoint. This prevents users from
uploading incorrectly-sized images for a specified image size.

Validation rules:
- 'original' size: must match original attachment dimensions exactly
- 'full' and 'scaled' sizes: requires positive dimensions only
- Regular sizes: dimensions must not exceed registered size maximums
  (with 1px tolerance for rounding differences)

Also updates existing tests to use appropriately-sized images and adds
two new test cases for dimension validation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add backport-changelog entry for Core PR #11100

Links Gutenberg PR #74903 (dimension validation for sideload endpoint)
to its corresponding WordPress Core PR.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix sideload filename filter to allow overwriting

Remove the file_exists check from filter_wp_unique_filename
so sideloaded sub-sizes can replace existing files. The
attachment name check is sufficient to prevent unintended
overwrites. Also consolidate the positive-dimension check
and fix translators comment argument order.

* Return early when wp_getimagesize() fails in sideload endpoint

Address review feedback: return a WP_Error when wp_getimagesize()
returns false (corrupted file, unsupported format) instead of silently
proceeding with zero dimensions. This also simplifies the downstream
code since $size is now guaranteed to be a valid array.

* Fix sideload tests to work with new dimension validation

Two tests had stale assertions after the switch to 'thumbnail'
size (still expecting 'medium'), and two newer trunk tests used
canola.jpg (640x480) in their sideload body, which now fails
dimension validation against medium (300x300) and thumbnail
(150x150). Use test-image.jpg (50x50) to fit within constraints.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Drag and Drop Drag and drop functionality when working with blocks [Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants