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 bugs fix include(autoCellHeight,multi grid drag,drag between grid two placeholder) #435

Closed
wants to merge 4 commits into from

Conversation

eagoo
Copy link

@eagoo eagoo commented Apr 24, 2016

1.auto cellHeight fix
2.tow gridstack acceptWidgets bugs fix
3.widget can not resize after drag between two grid bug fix
4.drag between tow grid,two placeholder fix

2.tow gridstack acceptWidgets bugs fix
3.widget can not resize after drag between two grid bug fix
4.drag between tow grid,two placeholder fix
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 71.536% when pulling ea52f62 on eagoo:master into 99653bd on troolee:master.

@ghominejad
Copy link

Thank you

@radiolips
Copy link
Member

I've been told that while this PR does fix the issues listed, it causes some other ones. Could anyone elaborate and/or set me straight? A couple parts seem a little iffy in the code, so an explanation would also be great.

@@ -800,6 +800,9 @@
.removeData('draggable')
.removeClass('ui-draggable ui-draggable-dragging ui-draggable-disabled')
.unbind('drag', onDrag);
if(el.find('div.ui-resizable-handle').length >0){
el.find('div.ui-resizable-handle').remove()
Copy link
Member

Choose a reason for hiding this comment

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

this for me fixes the issue where dragging a griditem to a second grid causes double style preventing resizing of it afterward.

@ghost
Copy link

ghost commented Sep 11, 2017

My team has verified the changes are needed to solve multiple grid issues like those mentioned in #718 . @eagoo could you please submit only the src/gridstack.js changes to the develop branch?

@radiolips
Copy link
Member

Also, please make sure that it passes all tests, including jscs. I see a couple spots in the code that won't match the style, but they're easy fixes. Simply run npm run build locally and you'll see those results.

@adumesny
Copy link
Member

adumesny commented Sep 11, 2017

note that "4.drag between tow grid,two placeholder fix" doesn't appear anymore in the latest 1.0.0-dev code (using two.html sample code) so not all those fixes are needed.
the only one I see and commented on the fix is #718 which is a 2 liner fix... I recommend we pick that (I'll be happy to provide a PR)
"1.auto cellHeight fix" I don't know what that issue is...

@radiolips
Copy link
Member

A few of these have been individually merged. @eagoo , could you split out the remaining fixes into individual PRs and provide jsfiddles or explanations of how to arrive at the issue / solution? Thanks so much!

@radiolips radiolips closed this Sep 11, 2017
@ADobrianskiy
Copy link

Hey, guys. Do you have any updates about the issue with duplication? It still reproduces on the latest version here: http://gridstackjs.com/demo/two.html
screencast: https://gfycat.com/evilspecificadder

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.

None yet

6 participants