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

Some cleanup and new functionality #83

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ThatRickGuy
Copy link
Contributor

Did a little clean up and added the NoGapBetweenItems parm. Since the spacer div is used to detect drag locations, it can't just be set to 0px height. This change adds a new div to behave the same but with a negative margin so that it doesn't consume space under the item.

@Postlagerkarte
Copy link
Owner

Postlagerkarte commented Nov 17, 2020

Hey @ThatRickGuy , I like the idea with making the gap between items more flexible.

Question is does it make sense to have a GapBetweenItems instead of the NoGapBetweenItems property and allow assigning values? If value = 0 ( same as NoGapBetweenItems) it will automatically apply the negative margin otherwise it would apply the value. This would also mean that there is no need for plk-dd-spacing and plk-dd-no-spacing , because the relevant values would be applied directly.

What are your thoughts on this?

@ThatRickGuy
Copy link
Contributor Author

Not a bad idea. I had set it as a negative boolean to avoid altering the default behavior and created the CSS class so that folks could alter it as they saw fit.

Setting it to an int would be easy enough, but how would you recommend injecting it? Inline style?

@Postlagerkarte
Copy link
Owner

Inline style seems to be reasonable. The spacing divs would still get assigned the "plk-dd-spacing" so if people really want the modify the inline style the could do so via !important.

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