jaumard / ngx-dashboard

Dashboard library for angular 4 and more
https://jaumard.github.io/ngx-dashboard/demo/demoDist/index.html
MIT License
71 stars 31 forks source link

Dashboard height is wrong when last line contain widget with height > 1 #13

Closed kirtiesl closed 7 years ago

kirtiesl commented 7 years ago

i am having dynamically created multiple widgets with different size. How to give Different size in responsive view. as its responsive view is disturbed.

jaumard commented 7 years ago

The widgets size are manage by units with [size]="[widget, height]" on widgets tag (example [size]="[2, 1]", you can choose the value of 1 unit by setting [widgetsSize]="[150, 150]" on the dashboard tag. Then when the dashboard is resized it recalculate the number of column possible to show.

nadir35 commented 7 years ago

Maybe my issue is related:

While dynamically changing the size[] of a widget, the dashboard doesn't adjust vertically.

Example: Lets say you have a dashboard with widgets of the sizes [2,2] and [1,1] and your dashboard width is 3. The Dashboard will only resize to this.widgetsSize[1] according to this snippet from ._positionWidget()

if (!items[index]) { var height = (row + 1) this.widgetsSize[1] + row this.margin; this._renderer.setStyle(this._ngEl.nativeElement, 'height', height + 'px'); return; }

Here we should use the tallest widget inside the dashboard to determine the dashboard height. Using row here is not quite right, since its the current row where a widget would be inserted. I was experimenting with using Math.max.apply(null,lines) on the lines-Array, which holds the height of each column (would be [2,2,1] in this case). However, I realized that during your positioning calculations you reset the lines-Array, so on adding another [1,1] widget would result in lines[] being [1,1,0] again instead of [2,2,2] which would work with my fix attempt.

Maybe you can change the calculation to not reset the lines-Array or introduce a new property to hold this value?

jaumard commented 7 years ago

Ho ok ! now I see the problem you're right :) I wasn't able to see the problem on the demo but by adding overflow:hidden on dashboard it's visible ^^

I'll take a look at it because it need some refactoring, lines is used to find if a position is free or not, it's not really the widget size so I need to change this part to apply the correct height. I'll do it asap :)

nadir35 commented 7 years ago

It also leads to some nasty loops right now:

Have a dashboard with a width of 2 [1,1]widgets. Have 3 [1,1] widgets in the first column and one [1,2] widget in the second. When you try to resize the middle [1,1] widget in the first column to lets say [2,2] the logic loops endlessly and keeps substracting from lines[] and adding rows. :O

I'll also see if I can find a fix since I need this till saturday ^^

jaumard commented 7 years ago

Ho never try to do stuff like that for the moment, I'll take care of the first problem tonight I think so don't waste time on this one. For the other one I need to setup an example to reproduce first so if you can look into this one or at least create a small example project to reproduce the problem it could be helpful

nadir35 commented 7 years ago

Good news: While failing to reproduce it in a example setup I realized that the loop was caused by myself. This was the cause:

ref.onWidgetGrow.subscribe(newSize => {
  ref.size = newSize;
  this.dashboard.ngOnChanges(null);
//this.dashboard.ngAfterViewInit();  <-- unneccessary , cause of loop
}
jaumard commented 7 years ago

@nadir35 that's it's a great news ! :D I was thinking I did shit lol cool ! I think I find a fix for the height error. Can you try this:

if (!items[index]) {
      let maxHeight = 0;
      for (let i = 1; i <= this._nbColumn && i <= items.length; i++) {
        const cmp = items[items.length - i].instance;
        if (maxHeight < cmp.size[1]) {
          maxHeight = cmp.size[1];
        }
      }
      const height = (row + maxHeight) * this.widgetsSize[1] + row * this.margin;
      this._renderer.setStyle(this._ngEl.nativeElement, 'height', height + 'px');
      return;
    }

to replace the beginning of _positionWidget method. Let me know if it works :)

nadir35 commented 7 years ago

Tried it. Almost works =)

image

However when i add another [1,1] widget to this it extends one more time for no reason: image

jaumard commented 7 years ago

Crap ^^ at least widgets are not cut now ^^ that's an improvement :) I'll search more latter then thanks for testing

jaumard commented 7 years ago

@nadir35 I think I made it, try with this:

if (!items[index]) {
      let remainingHeight = 0;
      for (let i = 0; i < lines.length; i++) {
        if (remainingHeight < lines[i]) {
          remainingHeight = lines[i];
        }
        lines[i]--;
      }
      if (remainingHeight > 0) {
        this._positionWidget(lines, items, index, column, row + 1);
      }
      else {
        const height = row * this.widgetsSize[1] + row * this.margin;
        this._renderer.setStyle(this._ngEl.nativeElement, 'height', height + 'px');
      }
      return;
    }

same to replace the beginning of _positionWidget method.

nadir35 commented 7 years ago

Nice, this seems to do the job.

jaumard commented 7 years ago

Cool :) I finalize few things and then I'll push this fix then. Thanks

jaumard commented 7 years ago

Please let me know if it's ok on last npm version now :) Thanks

nadir35 commented 7 years ago

Yes its ok on 1.1.2