Omit componentRef from DropAreaHOC's passed props

[adroitwhiz]

Resolves

Resolves #5642

Proposed Changes

This PR omits componentRef from the props that DropAreaHOC passes into its wrapped component.

Reason for Changes

If DropAreaHOC passes the componentRef prop into its wrapped component, this could potentially "clobber" alternative values of componentRef if the wrapped component is not expected to have a componentRef property.

This happened in the StageSelector component, which sets the componentRef of its child Box component and then sets all of the Box's remaining props to its own props. Because the wrapped StageSelector gained a componentRef prop out of nowhere, it was overwriting the Box's componentRef, breaking the DropAreaHOC's functionality and making it impossible to drag and drop anything onto the stage selector.

Test Coverage

Not sure what the best way to test dragging is.

Browser Coverage

Check the OS/browser combinations tested (At least 2)

Linux

• Firefox
• Chrome

Mac

• Chrome
• Firefox
• Safari

Windows

• Chrome
• Firefox
• Edge

Chromebook

• Chrome

iPad

• Safari

Android Tablet

• Chrome

Test Plan

• Description: Omit the componentRef React prop from the props passed into the component that the DropAreaHOC wraps
• Ensure that all existing uses of the DropAreaHOC are not broken
  • Test dragging and dropping into the backpack
    • Scripts
    • Costumes
    • Sounds
    • Sprites
  • Test dragging scripts from the backpack onto the scripts area
• Ensure that items can now be dragged and dropped onto the stage selector (to the right of the sprite pane)
  • Scripts
    • From backpack and the scripts area
  • Costumes
    • From backpack and the costumes tab of another sprite
  • Sounds
    • From backpack and the sounds tab of another sprites
• Be aware of existing behavior
  • You can drag a costume or sound from a target onto that same target, and it will duplicate it. Based on this comment, this behavior is intentional.
  • On Legacy Edge, the hover animation when you drag something onto the stage selector is broken. That bug was exposed by this PR but, as far as I can tell, is not related.
  • Dragging a script from the backpack onto the active sprite tile won't update the block workspace. This is a known bug.

[benjiwheeler]

@adroitwhiz Some things that would help move this PR forward:

1. any idea why the tests are failing?
2. Could you add a test plan? That is, as a reviewer, what should I try to verify that this is working, and hasn't broken related things? More here: https://github.com/LLK/scratch-ops/wiki/How-to-write-a-good-test-plan

[adroitwhiz]

1. The tests failed due to the "stale element" flakiness that only just got fixed today. I'll try force-pushing to rerun them and see if they either pass or fail for a reason actually related to this change.
2. Sure; I'll work on drafting one up!

[adroitwhiz]

@benjiwheeler I've added a test plan to the PR description; let me know if it's suitable!

[benjiwheeler]

I tested this, and it seems to work well. Thanks for the excellent testing plan, @adroitwhiz!

I still need to spend a little more time walking up and down the code tree to follow where componentRef is defined and used -- it all looks good so far, I just want to be thorough.

[benjiwheeler]

Took a look at the code and traced the props down the tree in a few places. This LGTM.

I tested a bunch of dragging functionality, but not everything. Could you please test all your cases, either before or after merging? (Could be easiest to test after it deploys automatically to staging, roughly half an hour after you merge it to gui develop)

[adroitwhiz]

Thanks for taking the time to look through the codebase! I'll test backpack dragging on staging after it's merged.

Is there any way to test backpack-related features locally? Logging into Scratch on the splash page from a locally-hosted scratch-www doesn't work, and navigating to the standalone /login page just redirects to the actual site.

[apple502j]

@adroitwhiz ?backpack_host=backpack.scratch.mit.edu&token=YOURTOKENHERE&username=adroitwhiz should work

[benjiwheeler]

I don't know if @apple502j's suggestion works -- if it does, that's cool!

I've never tried to get backpack working locally, but yes, it should be possible -- if you're OK using a production backpack (that's how we handle project saving locally -- we actually save to remote projects in production, just not on the standard S3 bucket)