-
Notifications
You must be signed in to change notification settings - Fork 187
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
Fixes error that appears in the console on drag nested resource to root #4150
Fixes error that appears in the console on drag nested resource to root #4150
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's still an error popping up, and it's visible in your video:
Only string ID format is supported
I see 2 drop events happening, where one ends up with an undefined/null target
. I think we can add defensive check that ignores any drops with a missing target
It looks like
A defensive check has been added to prevent execution of the drop in this instances. |
3b44d36
to
9b2aff0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
// Using `getCopyTrees` we can access the `excluded_descendants` for the node, such | ||
// that we make sure to skip copying nodes that aren't intended to be copied | ||
const trees = this.getCopyTrees(source.metadata.clipboardNodeId, true); | ||
if (payload.target) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In such scenarios where you have to nest a large block of code in a conditional, it can make more sense to have to shortcut the code early instead, which keeps the code flatter. For example:
if (!payload.target) return;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Blaine! I will most likely refactor this in my next studio PR
Summary
Description of the change(s) you made
This pr fixes a console error that is fired when dragging from a nested resource to its root directory
Manual verification steps performed
Screenshots (if applicable)
Screen.Recording.2023-06-16.at.22.54.06.mov
Does this introduce any tech-debt items?
No
Reviewer guidance
How can a reviewer test these changes?
References
Fixes #4112
Comments
Contributor's Checklist
PR process:
CHANGELOG
label been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocs
label has been added if this introduces a change that needs to be updated in the user docs?requirements.txt
files also included in this PRStudio-specifc:
notranslate
class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages
,components
, andlayouts
directories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarn
andpip
)