BTMorton / angular2-grid

A drag/drop/resize grid-based plugin directive for angular2
https://bmorton.co.uk/angular/
MIT License
354 stars 159 forks source link

Flexibility of the grid #89

Open Toktik opened 8 years ago

Toktik commented 8 years ago

Hey Ben, thanks for a great library.

We are using it in our startup. But we are facing some issues with the options and could not find best settings to fit our desired behaviour.

We have 2 root problems. 1) When we set a really small number for column width and row height and high number for max columns with auto resizing we get weird behaviour when dragging/resizing. Random widgets get randomly moved around the grid. Here is the settings: http://prntscr.com/b9vnbu interaction is also slow. We want small size for "cell"s, to be very flexible in terms of granularity. I can't really understand what's going under the hood, any hint would be helpful. 2) Is it possible to set minimum/maximum item size (col, row) per grid item?

Would be really great to get your feedback.

Thanks, Tigran

Diazbesjorge commented 8 years ago

Hi,

On top of what was mentioned by Tigran we would also like to implement some responsiveness into the grid without letting columns grow and shrink (we would prefer keeping column width fixed) but rather pushing everything that doesn't fit into one row into the next one and restoring everything to its original row when window is resized back to original dimensions.

It could be great if we could collaborate on these features. This is the final step to launch our dashboard app and we would prefer improving this library rather than transforming https://github.com/ManifestWebDesign/angular-gridster to angular 2.

We would really appreciate if you could give us some feedback in the short term.

Best regards, Jorge

BTMorton commented 8 years ago

Hey guys,

1) What kind of weird behaviour are you finding with the drag resize? I'll have a play and see if I can figure out what exactly is causing the issue. Do you want small steps but not small items, or small items and small steps?

2) Not as yet, no. I'm happy to open a new issue for that as a feature request, though!

3) You may have to run that by me again... So when you resize the page, items stay fixed size but re-arrange based upon the remaining space. There's a discussion there to be had about how items cascade/re-arrange themselves. I'm guessing you'd expect:

| 1 | 2 | 3 | 4 |
| 5 | 6 | 7 | 8 |

To become:

| 1 | 2 | 3 |
| 4 | 5 | 6 |
| 7 | 8 |

At the moment it would probably become:

| 1 | 2 | 3 |
| 5 | 6 | 4 |
        | 6 |
        | 8 |

Which is probably not what you want.... I have an open issue for adjusting how the cascade works (see #87) which this will probably link nicely to.

Getting it to go back to the previous positions would also take some thought. It's not as simple as just enumerating the items and managing positions based upon that, because gaps... Hmmm more thought required... Does gridster already handle this?

Diazbesjorge commented 8 years ago

Hi Ben,

Many thanks for your quick reply.

Let me first show you the dashboarding app that we are working on: http://prnt.sc/ba21qp We want to allow users to add the widgets that they desire and to be able to share their dashboard with others. We need to be able to relocate and resize widgets since we would like avoid having empty spaces.

1) We are aiming for big widgets as shown in the screenshot with increments as small as the cell size squared in red referenced there. The reason why we aim at this is because for table based widgets we can't go below the size shown in the screenshot, as that's as small as their rows can get. Since the number of rows depend on other settings (i.e. number of users) and we want to be able to avoid white spaces we would like to be able to resize other widget by multiples of that cell size. As for the weird behavior I will let Tigran elaborate on this.

2) This would be great as some of our widgets can't go beyond a certain minimum size.

3) It think that for our app this is the crux of usability. Think on what happens in this screencast https://giphy.com/gifs/xT8qB5N3Sc4n0TY6zu/fullscreen A user has created a nice compact dashboard resizing and aligning all widgets but suddenly he resizes the window and all sizes and locations are lost event when he restores the original window size. So I think two things are needed here: Defining a maximum column width (even when elements are pushed into the next line we might not want them to grow) and responsive relocation working in the following way:

a) User creates dashboard to fit his default screen size ("_" represents width of widget and empty spaces out of widget. "{}" represents window viewport size):

{|_1_|_2_|__3__|__4__|}
{|_5_|__6__|__7__|_8_|}

b) When screen size is reduced everything is rearranged in the following line as much as they fit, occupying more lines as needed (but without growing their size by implementing maximum_column_width):

{|_1_|_2_|__3__|_}
{|__4__||_5_|____}
{|__6__|__7__|___}
{|_8_|___________}

c) As the window is resized back all widgets would be restored to their original position:

{|_1_|_2_|__3__|__4__|}
{|_5_|__6__|__7__|_8_|}

d) Making the window bigger than original design size wouldn't have any effect except for having more empty space:

{|_1_|_2_|__3__|__4__|______}
{|_5_|__6__|__7__|_8_|______}

By the way, as for rows_height we didn't encounter these issues as we are planning on assigning max_rows=0 for infinite vertical scrolling.

As for gridster this seemed to work almost fine yesterday when we tried the latest version of angular-gridster but the problem is that we are using angular 2 and migratinb would take quite some effort. On the other hand we see that your library has a lot of potential and you have been quite active developing it in the last months.

Do you think implementing these features in angular2-grid would be possible? Do you have a rough estimate on the effort/time that it would take? We would be really happy to help. As you see we are a very active, although small, team and would love collaborating with you, but our deadlines are very tight now and we must make a move as soon as possible.

BTMorton commented 8 years ago

1) OK, I think I've figured out the weird behaviour. Was an issue with calculating the sizes within the grid items and how it checks against the minimums. That's fixed. I'll look into the speed issue, but I think that's due to the internal representation of this grid, especially at such small row/column sizes. I'll look into the other issues first, if that's ok with yourselves? Figure they're more urgent to you right now.

2) #90

3) OK, so I think there's a change of wording required on the demo. "Minimum Column Width" should be "Minimum Item Width" (and the same with height) which is why when the screen gets small, the items all resize. Also, in both of your configurations, you seem to be using the max columns value which auto updates the column width when you resize. From what you've described you definitely don't want that to be set. You probably want the min columns/rows value though, to prevent ridiculously small items. As I write this I realise you probably already figured that out, but just in case I'll leave it here :P

That is about what I expected. I would say that it is possible, but as to how long it will take, well I'm not good at estimates :P I need to figure out how it will handle different cases such as

|_1_|___|_2_|
|_3_|_4_|_5_|

should it go to:

|_1_|_2_|
|_3_|_4_|
|_5_|___

or

|_1_|___
|_2_|_3_|
|_4_|_5_|

and the bit I have to get my head round is how to convert the regular row/column state to the collapsed state. I'm thinking the best way is just to store a "current position" and a "configured position" and... actually that might not be too difficult... I'll keep you posted...

Diazbesjorge commented 8 years ago

Hey Ben, many thanks for your reply, you are awesome, it’s a pleasure working with you!

1) Excellent, Tigran will test this and give you some feedback. Agree with regard to speed, let’s look at that once the other issues are solved.

2) Thanks for creating #90. Count on us for the testing.

3) As for “max_columns” and “auto-resize” we are aware about the undesired auto-updating behavior but the problem is that with those off when the window is resized we get a horizontal scroll-bar which would also like to avoid. It seems like the solution is enabling a third mechanism intermediate to these two, which would work basically as we have discussed here: dynamically pushing widgets into the next row and recovering them as window is resized.

As for the last scenario that you mentioned:

|_1_|___|_2_|
|_3_|_4_|_5_|

My bet is that it should be handled as:

|_1_|___
|_2_|_3_|
|_4_|_5_|

Since apparently the user had good reasons to include an space between 1 and 2 I think we should not do “magic” removing those spaces. I think it should work as a conveyor belt, pushing into the next line what doesn’t fit into the current one.

Regarding the way to implement it I also think maintaining two positions is optimal. “Configured position” would the position defined by the dashboard designer for his viewport size, while “Current position” would be the position dependent on viewport size. If current viewport size is equal or bigger than the one the dashboard was designed for then current_position=configured_position, if current viewport is smaller current_position would be calculated as per the overflow algorithm described above.

Please let us know what you think and how we can help keeping this moving forward.

BTMorton commented 8 years ago

That's pretty much what I figured, on both counts (you knowing about the config and how to handle the gaps). If you can think of any other edge cases that might be worth considering, even better!

I probably won't bother with storing the viewport, it can be figured out by looking at the row/column config set. I think I've got enough to get started. I'll keep you guys posted :+1:

Diazbesjorge commented 8 years ago

The only other edge case I can think of right now is when widget is bigger than the viewport size. I think that in that case our overflow algorithm would try to accommodate by pushing one widget per row, but it would still show a horizontal scroll provided that the widget can not be resized (if auto_resize=off) neither broken into pieces.

Many thanks again Ben for all your support. We look forward to hearing from you in the following days.

Toktik commented 8 years ago

Hey Ben,

I've tested the 1) point. So far so good, your fix works. I can't reproduce the issue.

Also I've opened up 2 other PR fixes, which we were used in our project. Please take a look and tell me what you think.

Btw, how are you doing with other issues? We are almost ready to kick-off live our app except for this. Any update would be appreciated

BTMorton commented 8 years ago

Fantastic! Thanks for confirming. Have checked the PRs, one is fine but I'm not sure about the 2nd. I've left a comment in there.

Unfortunately, I don't have much free time during the week to work on this, but I'll try and get them sorted this weekend.

Diazbesjorge commented 8 years ago

Many thanks Ben, that would be great if you could address points 2 and 3 this weekend, that would mean we could launch our Beta release by Wednesday-Thursday. We will be online polishing other things, feel free to ping us for doing the testing.

BTMorton commented 8 years ago

OK, I've sorted min/max cols/rows for items and started on the framework for point 3. If I don't manage to sort the page shrinking stuff this weekend, at least you have a framework to kick off from!

Anywho, I had another couple of scenarios to run by you guys:

{|_1_|___2___|_3_|}

Should it go to:

{|_1_|_____}
{|___2___|_}
{|_3_|_____}

or:

{|_1_|_|_3_|}
{|___2___|__}

Also, this situation could be troublesome:

{|_1_|___|_2_|}
{|_3_|___|_4_|}

Should it be:

{|_1_|_2_|}
{|_3_|_4_|}

or

{|_1_|____}
{|_2_|____}
{|_3_|____}
{|_4_|____}

We can't put anything in the gaps or it will be cascaded ending up with:

{|_1_|_3_|}
{|_2_|____}
{|_4_|____}
Diazbesjorge commented 8 years ago

Hi Ben, I show you implemented #90, that's really great news for us. Thank you!

As for your scenarios above we must assume that the grid owner has designed it to look optimal at a certain minimum viewport size and he has some reasons to include all those empty spaces in between items (|___|). Although knowing those reasons would lead us towards one option or another on how to display in smaller viewports, the problem is that the grid doesn't know those reasons and hence we have to give a single solution for all the possible range of grid owner reasonings.

Based on this I would opt for implementing the most simplistic approach possible, let's call it "simple conveyor belt". In this approach all items and spaces would be maintained as they were initially designed but they would be pushed into the next line as the viewport shrinks. The effect would be very similar to what happens with a resizeable text area like this if we were to consider words as widgets http://www.w3schools.com/cssref/tryit.asp?filename=trycss3_resize

So back to your scenarios this is what I would chose using the "simple conveyor belt" approach. For the first one it would result in you proposal a:

{|_1_|_____}
{|___2___|_}
{|_3_|_____}

For the second one it would result in you proposal b:

{|_1_|____}
{|_2_|____}
{|_3_|____}
{|_4_|____}

Again under this option I am assuming that the grid designer has his reasons to include those gaps and hence we shouldn't just remove them. But there is a second option which we could call "condensed conveyor belt" that follows the same principle as above but removing all empty spaces when viewport width is smaller than original width. Under this option I would say that your first scenario should still result into proposal a (there are empty spaces shown but just because no widget fits in those empty spaces):

{|_1_|_____}
{|___2___|_}
{|_3_|_____}

While the second scenario would now result into your proposal a (empty spaces are used to fit the next widget in the conveyor belt):

{|_1_|_2_|}
{|_3_|_4_|}

I think both "simple conveyor belt" and "condensed conveyor belt" are valid and I would leave to you to decide which one is best.

Just one more note:

Hope this helps, feel free to bounce off any other questions that you come across.

BTMorton commented 8 years ago

OK, I've gone for the second method because it's just easier. Everything will just get squashed in where possible (basically...). It's currently live on my demo page, feel free to give it a go. Let me know any bugs you find ASAP and I'll try and fix them before your launch (no promises there though, I'm afraid). It still doesn't limit where items can be dropped at the moment. That's the next item on the list, but hopefully this will be enough to get you started.

Diazbesjorge commented 8 years ago

Hi Ben, I've seen your implementation and this is precisely what we were aiming for! Thank you very much, you really rock!

Apart from dropping order as mentioned in your comment (|_1_|__|_2_|_3_| being compressed as |_1_|_3_|_2_|) I see a few other minor issues that we can polish later. Let me nevertheless focus here on a couple of issues which I think are critical:

1) I understand that in order to get the desired "conveyor belt" effect with we must configure the following settings:

Auto-resize = false
Limit-to-screen = true
Max-columns = different from 0 in order not to have an unlimited canvas 

It works fine when column-width is larger than minimum-item-width (well, almost fine as it seems to allow to cut a bit off the last item: this http://prntscr.com/bcvsih results into this http://prntscr.com/bcvstb cuttting number 6 and showing a horizontal scroll bar) but if I design an scenario with minimum-item-width larger than column-width in order to have high granularity for size increments I don't manage to control when limit-to-screen is going to kick in. This is an example of grid design http://prntscr.com/bcvw7r and what happens when resizing http://prntscr.com/bcvwnc

2) The second issue that I see is when the mobile layout is kicked-in like in this example http://prntscr.com/bcvx3z as items get resized despite of having auto-resize=false. Under this situation I would expect items being pushed to as many lines as needed and complying with their sizing even if a horizontal bar has to be shown to scroll through their entire width.

Hopefully fixing just these two will enable us launching Beta. Thanks once again Ben!

Toktik commented 8 years ago

As per 1) point I think it is not relevant anymore. Since minimums are achieved by minCols and minRows per widget. We just don't need to set min_height and min_width.

2) Responsiveness still a question for me. I don't understand how the limit_to_screen works, but I do think it also tries to eliminate vertical scroll? Because I had 2 lines, reduced window vertically and the items moved above, which created horizontal scroll.

Diazbesjorge commented 8 years ago

Hi Tigran,

Did you check point 1) using minCols and minRows per widget? Did it work fine? My fear is that it will behave in the same way I described above (unfortunately I can't test this in the demo page).

As per your point 2) I think that limit_to_screen is mainly about trying to eliminate horizontal scrolls whenever possible (it is only not possible when the width for a single grid item is wider than the viewport width) so what you describe shouldn't happen.

Let's hope that Ben can take a look at my two issues above plus this new second issue that you mention.

BTMorton commented 8 years ago

The first point 2) The mobile layout is supposed to ignore sizes/layouts completely. The fact it's not, I'll need to look at... It's an optional CSS file so you probably don't have it, no worries :p

I'll look into the other issues. Note: you don't need to set a max columns value. Limit to screen will keep them within the bounds of the screen now.

Diazbesjorge commented 8 years ago

Hello Ben, thanks for you work during the last weeks. The grid works much better for us now, specially after the excellent performance improvement done as part of #102. We are now almost ready for a Beta public release of Kaizana.com, but there are however some grid issues that we would like to bounce off to see if you could finetune during the next days.Your help will be very very much appreciated.

  1. When using limit-to-screen there is the case that an item can be resized outside of the viewport borders. In this screenshot you see that we can resize but not place that same widget in the space (item shoudn't be placeable but it shouldn't be resizable beyond the viewport size either) https://giphy.com/gifs/l46CBLcnzHd8wATv2
  2. The desired conveyor effect works almost OK except when it fails. Pay attention to the blue "Keep Calm" item in this screenshot. On the first resize it recovers its position but it doesn't do so in the second one. https://giphy.com/gifs/xT8qBuU4z18J0N6HKg
  3. When the viewport is small enough our whole grid gets broken with items overlapping and growing out of their size. https://giphy.com/gifs/xT8qBngR6XHBIv26Ag
  4. When adding new items to the grid we would like to position them at the bottom of the grid, instead there are automatically added at the top pushing down whatever content we have there https://giphy.com/gifs/xT8qAY3sgAShHHsvQY We have tried setting prefer_new: false, but when done so the grid gets unresponsive.
  5. There is an slight issue when resizing items below their minimum size. I don't know if you can appreciate in the following screencast but as the item border is pulled in the content temporarily outgrows its borders until the mouse is released an the item size is restored to its minimum https://giphy.com/gifs/3o72ETLSgImrUp8niU
BTMorton commented 8 years ago

Hey Jorge, apologies for taking a while to respond. I don't have as much time as I'd like to work on this, so sorry about that.

RE: 1 and 2, I'm currently working on a better implementation of the conveyor which will help to deal with these issues.

  1. It looks like the overlap/grow is due to the content of your item. If you set overflow: hidden on the item, it should prevent that, though, I can't say why your items are being expanded like that internally. The size of the items shouldn't be (and don't look to be) changing. Feel free to inspect using dev tools and let me know if they are.

  2. Yes, if prefer_new is set to true and you add an item with {col: 1, row: 1} it will take that spot. I'll look into the prefer_new: false issue, but I think that's what you'll want.

  3. Again, if you disable overflow it will fix that issue. The minWidth and minHeight properties prevent dragging below a certain item size, which I imagine you'll want to be able to set per item, similar to the minCol/minRow. I can look into adding that as well.

Thanks for sticking with me, I'll try and get to the above this weekend. Any more issues, please do let me know.

BTMorton commented 8 years ago

OK, I have updated the limit grid function and made it use a bin packing-style algo. In all of my (admittedly limited) tests it does the job, better than before. Give it a go and see if it fixes your problems :)

Diazbesjorge commented 8 years ago

Thanks Ben, we're going give a try to your new version. Were you also able to look into issues related overflow:hidden, prefer_new:false and being able to set minWidth / minRow per item?

Diazbesjorge commented 8 years ago

Hi Ben, we tested it and it is working much better now. We also see that prefer_new:false is working well now. Let me just recap the issues that we see pending:

  1. There is one scenario in which the grid doesn't recover it original design after resizing. I think it is related to having a tall item in the middle of a row (not at the end of the row which works well) Take a look at items 3 and 4 in this gif https://giphy.com/gifs/l41YuEFVxdS1Lmtqw
  2. Being able to set minWidth / minRow per item would solve our overflow problems.

By the way, we also noted that for the limit-to-screen setting to work well Cascade Direction can't be set to "off". We changed it to "up" with no problem, but just for you to know.

BTMorton commented 8 years ago

Yes, at the moment limit to grid only works with cascade up. For the time being I have no intention to changing that because getting it work as it does know has caused me much headache and I have other things I want to be doing :P

I never encountered any issues with prefer_new: false so I don't know what was going on there.

Will keep you updated on the other bits

BTMorton commented 8 years ago

Whoops, nice little logic mistake there! Should be fixed now, and I've added minWidth/minHeight in for you too. Give it a go and hopefully we're good!

Diazbesjorge commented 8 years ago

Excellent, it works almost to perfection now. After extensive testing I found that in all cases all items recovered their original grid position :-) I just found a case in which items will leave some weird empty space when resizing. Take a look at item number 4 in this gif https://giphy.com/gifs/3o6gE6vPNmV71P5Zkc

We'll also test minWidth/minHeight and will let you now.

Finally I have an open question. When should the new grid configuration be considered as new target? We have two options:

  1. Right now when viewport is resized all elements are considered fluid and recover their initial position when viewport is resized back, except for those elements that are repositioned by the user. For those repositioned items the dashboard considers them as new target positions and hence won't recover their initial position, but the other untouched items will recover their position resulting in a kind of mixed situation.
  2. An alternative would be considering that if the user repositions at least one item his real intention is to design a new whole target dashboard and hence we should consider all the new item positions as new target, so we shouldn't recover any other position for any item beyond this dashboard design.

I am in favor for the logic of scenario 2, mainly due to the fact that most always if you reposition a single item, trying to recover the original dashboard design as per scenario 1 will result in a mix of old and new positions that won't make sense. At least in scenario 2 you know that if you adjust something this whole dashboard is your new target and you get something that still makes sense. What do you think?

BTMorton commented 8 years ago

Cool, thanks for that. Fix is pushed now. Will be live on the demo site ASAP (currently writing this on a train).

At the moment it's probably closer to the first scenario. What will happen at the moment is that the moved/resized item and any items that get cascaded because of the move will be saved to there too. Unfortunately, at the moment the cascade effect saves when it happens, which means if you move an item, other items reshuffle, and then you put the first item back, they will all be saved with the new positions. Does that make sense? To change to scenario 2, it'd just be a case of updating the saved positions of the items on drag/resize stop if that's the optimal solution? I could probably do it with a new config option saveAllOnDragStop which effectively toggles between the two options, but would have very little effect on the general running of the grid. I'll wait to hear from you guys on that one.

Edit: Now live on the demo site. Feel free to have a play :)

Diazbesjorge commented 8 years ago

Yes, I think that the fact that moving an item does affect other cascaded items but not the items above, which will be the only ones returning to their original position upon resizing, is producing a nonsense dashboard result.

What I have experienced with my test users is that when decreasing the window size and experiencing the result of limit-to-screen they feel tempted to adjust a widget here or there, but this has terrible consequences when dashboard is restore to original image. This may also happen accidentally, and I might even have to consider including a switch in the dashboard settings so that users disable accidental item re-positioning.

In any of the cases I do consider that scenario 2 is best for users. If you are considering doing it with a config option that's even better, although I can't honestly think of any use case in which scenario 1 would be preferred. As for your comment below I don't really get how you are intending to implement it

To change to scenario 2, it'd just be a case of updating the saved positions of the items on drag/resize stop if that's the optimal solution?

but I think you got it, the idea is that upon resizing/moving at least one of the items, all the other items would save their position as if this was the new target position selected by the user for every single one of them. This means that further increasing the viewport width wouldn't result in any item movement, while reducing it would result in the expected conveyor belt effect.

Diazbesjorge commented 8 years ago

Hi Ben, just checking in how everything is going. I am aiming for a beta release this week, do you have availability to solve the point above plus two new issues that were discovered? Let me do a summary:

  1. When limit-to-screen is enabled, adjusting a single item should result in the whole grid design being saved as described in my last comment above.
  2. When limit-to-screen is enabled, the conveyor belt effect (items moving from one line to the next when there's not enough viewport width) is not kicking in upon first dashboard load, meaning it doesn't work unless window is resized: https://giphy.com/gifs/l46ClSwy862vg2dOw
  3. Sometimes my grid doesn't maintain it desired design after reloading. Take a look at the two top left items and the chart just below them https://giphy.com/gifs/3o6Zt9BlQScsUiDlw4

I would be great having your feedback. Thanks!

Diazbesjorge commented 8 years ago

Hello Ben,

We remain using our app with a private group of users but would like to move into public Beta release as soon as these pending issues are solved. Please find all the information about our Kaizana app at http://kaizana.com/ live now for the first time.

Could you help us solving the issues summarized in my last comment above?

Many thanks in advance!