Try avoiding the deprecated findDOMNode API from DropZone Provider by youknowriad · Pull Request #11168 · WordPress/gutenberg

youknowriad · 2018-10-28T09:20:24Z

While I don't like adding DOM nodes for nothing (we can probably avoid these nodes if we use hooks), I don't see another way to remove findDOMNode and keep the same functionality.

Thoughs @aduth @gziolo

aduth

This breaks "click below to add" behavior, because of the added div (the WritingFlow relies on 100% height of its full ancestry)

aduth · 2018-10-29T13:27:08Z

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

 		const dragEventType = getDragEventType( event );
 		const dropZone = this.dropZones[ hoveredDropZone ];
-		const isValidDropzone = !! dropZone && this.container.contains( event.target );
+		const isValidDropzone = !! dropZone && this.container.current.contains( event.target );


If this is the only instance of the DOM reference, could we just have onDrop be bound as an event handler to the wrapping div? I guess the main difference is the lack of stopPropagation / preventDefault on all other drops on the page, which I could see as equally "correct" and "annoying" (in cases of mis-drops causing navigation).

Also, with use of Slot/Fill, using DOM Element#contains isn't guaranteed to be accurate anyways.

If this provider is at the top-level, is there really any point to checking this at all?

Assuming we're tracking hover for the relevant dropZone, that seems to be an extra (better?) level of certainty to where the drop is taking place.

Yes, there is, this fixes a bug where drag and dropping to the media library creates the image twice. (It's also caught by the dropzones in the editor even if the the dragged item is outside the editor)

Do you recall the issue / pull request for it? This sounds a bit fishy...

Makes me wonder if there's possibly something going on with StrictMode double-rendering (#6466 (comment))

So the original PR fixing this issue is this one #5432

but I recall it being refactored to use the container technique in another PR. Looking

Looks like it was introduced here #4115 but I don't recall the exact reasons :( Will investigate more.

So yes, in my testing, it is still necessary to avoid the double upload issue.

aduth · 2018-10-29T13:33:43Z

There was meant to be an E2E test covering the broken behavior. Apparently it's not working so well.

Originally: https://github.com/WordPress/gutenberg/pull/5991/files#diff-708f5078fca40c1d3de2c973ddc5e255

Current:

gutenberg/test/e2e/specs/adding-blocks.test.js

Lines 17 to 18 in a2f81fa

    
           // Click below editor to focus last field (block appender) 
        
           await page.click( '.editor-writing-flow__click-redirect' );

youknowriad · 2018-11-01T10:59:18Z

I updated the PR. I fixed the e2e test to break properly if needed. And I also fixed the issue using CSS, I don't have a better alternative at the moment.

aduth · 2018-11-01T13:55:41Z

Aside: I really wish display:contents was better supported. It would be perfect for these sorts of wrappers.

aduth · 2018-11-01T14:09:29Z

Maybe I'm missing something? I don't see the issue resolved by #5432 with c869c81.

youknowriad · 2018-11-01T14:17:51Z

@aduth I suspect it's because the drop event is triggered in that case because the div container is rendered behind the modal. I'm checking the event.target which is a bit different.

youknowriad · 2018-11-01T14:22:04Z

Or are you saying it's fixed? Because I can't reproduce in my testing, your commit seems fine.

aduth · 2018-11-01T16:53:14Z

Right, c869c81 is what I was trying to suggest, and appears to work well enough. Will approve if it's agreeable to you.

…rnmobile/port-quote-block-step-1 * 'master' of https://github.com/WordPress/gutenberg: (22 commits) Add removed periods to block descriptions. (#11367) Remove findDOMNode usage from the inserter (#11363) Remove deprecated componentWillReceiveProps from TinyMCE component (#11368) Create file blocks when dropping multiple files at once (#11297) Try avoiding the deprecated findDOMNode API from DropZone Provider (#11168) Fix: make meta+A behaviour of selecting all blocks work on safari and firefox. (#8180) Remove _wpGutenbergCodeEditorSettings and wp.codeEditor assets (#11342) Remove the Cloudflare warning (#11350) Image Block: Use source_url for media file link (#11254) Enhance styling of nextpage block using the Hr element (#11354) Embed block refactor and tidy (#10958) Nonce Middleware: Wrap the nonce middleware function into it's own function that isn't regenerated on every API request. (#11347) Fix RTL block alignments (#11293) RichText: fix buggy enter/delete behaviour (#11287) Remove code coverage setup (#11198) Parser: Runs all parser implementations against the same tests (#11320) Stop trying to autosave when title and classic block content both are empty. (#10404) Fix "Mac OS" typo + use fancy quotes consistently (#11310) Update documentation link paths (#11324) Editor: Reshape blocks state under own key (#11315) ... # Conflicts: # gutenberg-mobile

Links the finalize endpoint core backport PR to Gutenberg PR #74913 for client-side media processing.

#74913) * Add hooks and extension points for client-side media processing Adds the following hooks and extension points to ensure existing media hooks continue to run when client-side media processing is active: PHP: - Finalize REST endpoint (POST /wp/v2/media/{id}/finalize) that triggers wp_generate_attachment_metadata filter after all client-side operations complete, so server-side plugins can post-process attachments. - Preload paths for image_sizes and image_size_threshold fields. JS: - editor.media.imageQuality hook to filter image quality for resize ops. - editor.media.imageSizesToGenerate hook to filter which thumbnail sizes are generated client-side. - Finalize operation type appended to the image upload pipeline after thumbnail generation, with gating to wait for child sideloads. - Parent trigger so finalization runs after all sideloads complete. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * remove preload change * update docblock * Remove client-side imageSizesToGenerate filter Image sizes are already filterable server-side via intermediate_image_sizes_advanced and related hooks. * Add tests for editor.media.imageQuality filter Verifies the filter is called with the default quality value and correct context during resizeCropItem, and that it is skipped when no resize args are provided. * Add hooks reference to upload-media tsconfig The test file imports @wordpress/hooks, so the tsconfig needs a project reference to satisfy lint:tsconfig. * Add backport changelog for core PR #11168 Links the finalize endpoint core backport PR to Gutenberg PR #74913 for client-side media processing. * Replace imageQuality JS filter with store setting The applyFilters hook runs per-upload and is hard to remove once added. Use the existing updateSettings/getSettings store pattern instead, which is a one-time operation and consistent with how other image settings are managed. * Move finalizeItem API call out of upload-media The upload-media package is implementation-agnostic and should not make direct REST API calls. This moves the apiFetch call to a new mediaFinalize utility in the editor package and injects it via the existing settings pattern, matching how mediaUpload and mediaSideload already work. * Add tests for finalizeItem and mediaFinalize Verify the injected callback pattern works correctly: finalizeItem calls the setting, handles missing callbacks, and recovers from errors. mediaFinalize hits the correct REST endpoint. * Update package-lock.json after removing api-fetch dep * Update lib/media/class-gutenberg-rest-attachments-controller.php Co-authored-by: Weston Ruter <weston@ruter.net> * Update lib/media/class-gutenberg-rest-attachments-controller.php Co-authored-by: Weston Ruter <weston@ruter.net> * Remove unreachable sizesToGenerate check The outer condition already guarantees missing_image_sizes.length > 0, making the sizesToGenerate.length === 0 block dead code. * Rename finalizeUpload to mediaFinalize Aligns with the existing mediaUpload and mediaSideload naming convention used in block editor settings. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Weston Ruter <weston@ruter.net>

youknowriad requested a review from a team October 28, 2018 09:20

youknowriad added the [Type] Code Quality Issues or PRs that relate to code quality label Oct 28, 2018

aduth requested changes Oct 29, 2018

View reviewed changes

aduth mentioned this pull request Oct 29, 2018

Allow margins to collapse & refactor block toolbar #7365

Merged

Try avoiding the deprecated findDOMNode API

6a049e5

youknowriad force-pushed the try/remove-find-dom-node branch from aa6040a to 6a049e5 Compare November 1, 2018 10:58

youknowriad requested a review from aduth November 1, 2018 10:59

youknowriad mentioned this pull request Nov 1, 2018

Upgrade to React 16.6 stable APIs #11360

Closed

12 tasks

Components: Use event handler in place of ref

c869c81

aduth approved these changes Nov 1, 2018

View reviewed changes

youknowriad merged commit 506187b into master Nov 1, 2018

youknowriad deleted the try/remove-find-dom-node branch November 1, 2018 17:06

youknowriad added this to the 4.3 milestone Nov 2, 2018

adamsilverstein added a commit that referenced this pull request Mar 10, 2026

Add backport changelog for core PR #11168

1ab2a13

Links the finalize endpoint core backport PR to Gutenberg PR #74913 for client-side media processing.

Conversation

youknowriad commented Oct 28, 2018

Uh oh!

aduth left a comment

Choose a reason for hiding this comment

Uh oh!

aduth Oct 29, 2018

Choose a reason for hiding this comment

Uh oh!

aduth Oct 29, 2018

Choose a reason for hiding this comment

Uh oh!

aduth Oct 29, 2018

Choose a reason for hiding this comment

Uh oh!

youknowriad Oct 29, 2018

Choose a reason for hiding this comment

Uh oh!

aduth Oct 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowriad Oct 29, 2018

Choose a reason for hiding this comment

Uh oh!

youknowriad Oct 29, 2018

Choose a reason for hiding this comment

Uh oh!

youknowriad Nov 1, 2018

Choose a reason for hiding this comment

Uh oh!

aduth commented Oct 29, 2018

Uh oh!

youknowriad commented Nov 1, 2018

Uh oh!

aduth commented Nov 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aduth commented Nov 1, 2018

Uh oh!

youknowriad commented Nov 1, 2018

Uh oh!

youknowriad commented Nov 1, 2018

Uh oh!

aduth commented Nov 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aduth Oct 29, 2018 •

edited

Loading

aduth commented Nov 1, 2018 •

edited

Loading