vpusher / the-grid

Grid layout custom element with drag and drop capabilities
https://www.webcomponents.org/element/vpusher/the-grid
MIT License
129 stars 31 forks source link

Tiles Overlap #1

Closed mario-aleo closed 7 years ago

mario-aleo commented 7 years ago

Hello,

First: Nice job, this component is awesome :DD Ok so, the ability to drag and resize tiles is great but there's just one problem, the tiles can overlap each other on drag or resize, is there anyway to make the tiles to auto reorganize so they wont overlap?

Thanks, Mario Aleo

vpusher commented 7 years ago

@mario-aleo thank you, I appreciate it. You are totally right. In fact, the next planned features for this component are responsiveness and overlapping handling. Even if the first seems easy and should come in the next release, the second one is far more complex.

What I need, is to find a convincing behavior so that the tiles re-organize themselves naturally. This is the harder part. I know some implementations, such as gridster.js, which have that kind of force always moving the tiles to the top. I don't know yet if it's the way that we want to use a grid based component...

As a path finder, I will probably deliver a first version which will enable the placeholder to always fall into the nearest empty position (so there is no overlap during d&d). Then, I would be able to deliver a second version with that auto reorganization algorithm.

Feel free to give your opinion or any input on that topic.

bkjohnson commented 7 years ago

@vpusher I was going to write out a rough algorithm and explain it but decided to test it myself and see if it worked. It basically comes down to:

    Don't update the `placeholder` row & col if the `player` has a collision

I didn't submit a PR because it is just a quick sample and the code would need to be improved (Edit: It would be nice to provide a CSS mixin to use when an overlap happens), but you can see the basic changes here: https://github.com/bkjohnson/the-grid/commit/af25d0c3002f9a372d096009829f3f4a39c52cb1

This is what it looks like in action:

nooverlap

Do you think this approach is a good starting point? Happy to hear any feedback.

vpusher commented 7 years ago

@bkjohnson yes this basically the first version I was talking about. One remark btw, your implementation is a bit costful since it computes the client rect at each move. We could manage to detect collisions only using the tile's attributes. Everything we need is already there.

bkjohnson commented 7 years ago

Good point!

Also, looking at every single tile anytime one of them is being dragged isn't very efficient and might become noticeably slower as the list of tiles grows. It could be improved by using a tree so that you are looping through a (potentially) much smaller list of tiles. Something like this could be useful.

This also raises another issue though. Imagine a scenario where the grid is small and/or there are so many tiles placed at irregular intervals that there isn't an empty space large enough for them to move to. Even a reorganization algorithm wouldn't work. With this in mind, do you think it would be good if <the-grid> had an optional infinite property? For example, if infinite, anytime a tile gets dragged to an edge of the grid more rows/columns are automatically added. I know that I will be using a feature like this, but I'm not sure if it belongs in my own wrapper component or in <the-grid> itself.

vpusher commented 7 years ago

@bkjohnson You are talking about column and row auto spawning. That would be an interesting feature but I'm wondering why would you make them spawn on the fly if you can render them before by asking more columns/rows through the element API (i.e attributes) ?

By the way, if you increment rowCount, or colCount, the grid should re-render itself (even using the dom attributes).

Let's keep this issue for tile's overlap and please open a new one as feature request so we can keep talking about it.

yorrd commented 7 years ago

@bkjohnson @vpusher I've made a pull request concerning the auto spawning feature. See https://github.com/vpusher/the-grid/issues/10 // https://github.com/vpusher/the-grid/pull/11

vpusher commented 7 years ago

See v1.1.0