wynfred / ngx-masonry

Angular Module for displaying a feed of items in a masonry layout
MIT License
157 stars 52 forks source link

Change array order breaks masonry template 🤔 #93

Closed AlonsoK28 closed 2 years ago

AlonsoK28 commented 2 years ago

My app consist in an array of item that can be ordered by some filter criteria using a MAT MENU to select the preferred criteria

image

But when I change the order or my items array masonry is rendered completely wrong, for example:

image

I have tried as documentation says:

Set [ordered]="true" in template, also use masonry functions to render again when my array is re-ordered

this.masonry.reloadItems();
this.masonry.layout();

This is how it looks ordered at init

image

Some times the order is applied fine but mostly is rendered wrong

This is my reproduction repo in github https://github.com/AlonsoK28/amazmation-app-2/tree/feature/update-layout because I cant upload it to stackblitz 😢

I hope you can help me 😜👏

wynfred commented 2 years ago

Hi, it seems like the reloaditems function is causing the problem. Try remove both reloadItems and layout

  showMoreImages() {
    this.itemsLimit += 15;
    this.tempMasonryItems = this.orderPipe.transform(this.offersFromService, this.orderByPipeConfig.expression, this.orderByPipeConfig.reverse, this.orderByPipeConfig.caseInsensitive, this.orderByPipeConfig.comparator).slice(0, this.itemsLimit);
    this.currentMasonryItems = this.tempMasonryItems;
    // this.masonry?.reloadItems();
    // this.masonry?.layout();
  }

  setCurrentOrderBy(el: OrderByDefinition){
    this.orderByPipeConfig = el.config;
    // order all items in array, not only 15
    this.itemsLimit = 15;
    this.tempMasonryItems = this.orderPipe.transform(this.offersFromService, this.orderByPipeConfig.expression, this.orderByPipeConfig.reverse, this.orderByPipeConfig.caseInsensitive, this.orderByPipeConfig.comparator).slice(0, this.itemsLimit);
    this.currentMasonryItems = [];
    this.currentMasonryItems = this.tempMasonryItems;
    // this.masonry?.reloadItems();
    // this.masonry?.layout();
  }
AlonsoK28 commented 2 years ago

Hi, it seems like the reloaditems function is causing the problem. Try remove both reloadItems and layout

  showMoreImages() {
    this.itemsLimit += 15;
    this.tempMasonryItems = this.orderPipe.transform(this.offersFromService, this.orderByPipeConfig.expression, this.orderByPipeConfig.reverse, this.orderByPipeConfig.caseInsensitive, this.orderByPipeConfig.comparator).slice(0, this.itemsLimit);
    this.currentMasonryItems = this.tempMasonryItems;
    // this.masonry?.reloadItems();
    // this.masonry?.layout();
  }

  setCurrentOrderBy(el: OrderByDefinition){
    this.orderByPipeConfig = el.config;
    // order all items in array, not only 15
    this.itemsLimit = 15;
    this.tempMasonryItems = this.orderPipe.transform(this.offersFromService, this.orderByPipeConfig.expression, this.orderByPipeConfig.reverse, this.orderByPipeConfig.caseInsensitive, this.orderByPipeConfig.comparator).slice(0, this.itemsLimit);
    this.currentMasonryItems = [];
    this.currentMasonryItems = this.tempMasonryItems;
    // this.masonry?.reloadItems();
    // this.masonry?.layout();
  }

I will try 😎😎😎😎

AlonsoK28 commented 2 years ago

Hi, it seems like the reloaditems function is causing the problem. Try remove both reloadItems and layout

  showMoreImages() {
    this.itemsLimit += 15;
    this.tempMasonryItems = this.orderPipe.transform(this.offersFromService, this.orderByPipeConfig.expression, this.orderByPipeConfig.reverse, this.orderByPipeConfig.caseInsensitive, this.orderByPipeConfig.comparator).slice(0, this.itemsLimit);
    this.currentMasonryItems = this.tempMasonryItems;
    // this.masonry?.reloadItems();
    // this.masonry?.layout();
  }

  setCurrentOrderBy(el: OrderByDefinition){
    this.orderByPipeConfig = el.config;
    // order all items in array, not only 15
    this.itemsLimit = 15;
    this.tempMasonryItems = this.orderPipe.transform(this.offersFromService, this.orderByPipeConfig.expression, this.orderByPipeConfig.reverse, this.orderByPipeConfig.caseInsensitive, this.orderByPipeConfig.comparator).slice(0, this.itemsLimit);
    this.currentMasonryItems = [];
    this.currentMasonryItems = this.tempMasonryItems;
    // this.masonry?.reloadItems();
    // this.masonry?.layout();
  }

I have tried to do it that way.

When I remove the "reloadItems()" and "layout()" functions, it appears that the graphical errors are no longer displayed BUT the ordering of the elements is not correctly represented in the TEMPLATE.

For example, the ordering by price from highest to lowest is not correct, also when ordering by other criteria such as the property "rating", "name" and others, the data is displayed incorrectly.

I think that the function "reloadItems()" and "layout()" is necessary, although I don't know why it causes the graphical error.

And I tried to use the "setTimeout()" technique as you have commented in answers of other incidents but I have not been able to make it work either.

wynfred commented 2 years ago

Hi, didn't realize the order is wrong. Could you try this? Thanks!

  setCurrentOrderBy(el: OrderByDefinition){
    this.orderByPipeConfig = el.config;
    // order all items in array, not only 15
    this.itemsLimit = 15;
    let tempMasonryItems = this.orderPipe.transform(this.offersFromService, this.orderByPipeConfig.expression, this.orderByPipeConfig.reverse, this.orderByPipeConfig.caseInsensitive, this.orderByPipeConfig.comparator).slice(0, this.itemsLimit);
    this.currentMasonryItems = [];
    setTimeout(() => {
      this.currentMasonryItems = tempMasonryItems;
    });
  }
AlonsoK28 commented 2 years ago

Owner

I've tried this solution and looks like works for now 🤗🤗🤗