Skip to content
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

Upgrade conditional #26

Merged
merged 8 commits into from
Nov 8, 2022
Merged

Upgrade conditional #26

merged 8 commits into from
Nov 8, 2022

Conversation

ayodeji-ti
Copy link

No description provided.

Copy link

@vinayak-sachdeva vinayak-sachdeva left a comment

Choose a reason for hiding this comment

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

Can we remove Box under Conditional in Sidebar?

{props.show === 'show-both' || props.show === undefined ? (
<>
<ComponentPreview componentName={component.children[0]} />
<ComponentPreview componentName={component.children[1]} />

Choose a reason for hiding this comment

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

what if I delete any box

Copy link
Author

Choose a reason for hiding this comment

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

the box gets deleted as expected, and only one box is displayed, if both are deleted then nothing is displayed, no error is thrown, but I'll add the null check

Copy link
Author

Choose a reason for hiding this comment

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

Also I tried removing the box under conditional, but it ceases to become a meta component when I do that, and the conditional component ends up having no box in it by default

Choose a reason for hiding this comment

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

Okay

}

return (
<Box pos="relative" {...props} ref={drop(ref)}>

Choose a reason for hiding this comment

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

why do we need ref for outerbox

Copy link
Author

Choose a reason for hiding this comment

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

So that we can move the conditional component, if we don't add the ref, then asides from the initial drag and drop, we won't be able to move it again

Choose a reason for hiding this comment

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

But that means it can accept other draggable items as well, which is not desired. Try to avoid that

Copy link
Author

Choose a reason for hiding this comment

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

No, only boxes, if you check the conditional preview, you would see that I have added the accepted type as Box, any other component won't be accepted, you can try it out for yourself
Screenshot (399)

Choose a reason for hiding this comment

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

Okay, great

@ayodeji-ti ayodeji-ti merged commit 73b3a86 into master Nov 8, 2022
@ayodeji-ti ayodeji-ti deleted the upgrade-conditional branch November 8, 2022 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants