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

onPageChange event returns different event data (infinite scroll with lazy loading feature?) #116

Closed
zxshinxz opened this issue Sep 13, 2016 · 6 comments

Comments

@zxshinxz
Copy link

Hi, all

In the document
document

onPageChange: Triggered when the data-table is paged. Arguments: offset, limit, pageCount

but it returns just page value for now

DataTable.ts
onPageChanged(action) { this.state.setPage(action); this.onPageChange.emit(action.value); }

Just wondering, What if we could just make DataTable.ts subscribe event from State Service to match documentation?

DataTable.ts

  ngOnInit(): void {
    const {options, rows, selected} = this;

    this.sub = this.state.onPageChange.subscribe((action) => {
      this.onPageChange.emit(action);
    });

    this.state
      .setOptions(options)
      .setRows(rows)
      .setSelected(selected);
  }

  onPageChanged(action) {
    this.state.setPage(action);
  }

Statets


 setPage({type, value}) {
    if (this.options.offset !== (value - 1)) {
      this.options.offset = value - 1;

      this.onPageChange.emit({
        type,
        offset: this.options.offset,
        limit: this.pageSize,
        count: this.rowCount
      });
    }
  }

and

this might allow angular2-data-table to support infinite scrolling with lazy-loading by
emitting "onPageChange" event from "Body.ts" onScroll

Body.ts


 onBodyScroll(props) {
    this.state.offsetY = props.scrollYPos;
    this.state.offsetX = props.scrollXPos;

    this.updatePage(props.direction);
    this.updateRows();
  }

  updatePage(direction) {
    const idxs = this.state.indexes;
    let page = idxs.first / this.state.pageSize;

    if(direction === 'up') {
      page = Math.floor(page);
    } else if(direction === 'down') {
      page = Math.ceil(page);
    }

    if(direction !== undefined && !isNaN(page)) {   
      // pages are offset + 1 ;)
      this.state.setPage({                           <-- emits "onPageChange" event
        type: 'body-event',
        value: page + 1
      });
    }
  }


I currently have angular2-data-table working version of infinite scrolling with lazy-loading (not optimized yet) which also allow pagination at the same time.

any thoughts?

infinite-scroll-demo

Thanks for the great library 👍
Andy

@amcdnl
Copy link
Contributor

amcdnl commented Sep 13, 2016

Agreed, the API should be updated to match.

Whats the advantage of the other change? I believe you can accomplish what you described currently.

@zxshinxz
Copy link
Author

zxshinxz commented Sep 13, 2016

After firm look at the source, I actually found real reason why it was
returning different data.

In DataTable.ts

  constructor(public state: StateService,
              element: ElementRef,
              differs: KeyValueDiffers) {

    this.element = element.nativeElement;
    this.element.classList.add('datatable');
    this.onPageChange = this.state.onSelectionChange;   <--- typo

    this.rowDiffer = differs.find({}).create(null);
    this.colDiffer = differs.find({}).create(null);
  }

should be

DataTable.ts

  constructor(public state: StateService,
              element: ElementRef,
              differs: KeyValueDiffers) {

    this.element = element.nativeElement;
    this.element.classList.add('datatable');
    this.onPageChange = this.state.onPageChange;   <--- should be

    this.rowDiffer = differs.find({}).create(null);
    this.colDiffer = differs.find({}).create(null);
  }

"Whats the advantage of the other change?"
If you are asking about infinite scrolling, (up to my knowledge of this library, so far)
I found it supports virtual scrolling not infinite scrolling. Virtual scrolling demo I encounter in this repo all loaded 100k.json once and bound page size rows in Body.ts to render in the grid body which 100k.json isn't that small if you load it from server-side. It should load portion of the data from the server at a time when required.

This might be due to the bug:
broken linkage "this.onPageChange = this.state.onPageChange" because "onBodyScroll()" function calls "updatePage()" which eventually calls "onPageChange()"

Even if above bug gets fixed, invoking onPageChange event on onBodyScroll event is not an good idea I think. (May be call onPageChange only when page changes?)

In my opinion, better Idea would be splitting onPageChange event and loadData event (need better name than this)

where -
onPageChange: gets invoked when pager changes current page state.
loadData: preload the data from datasource(most likely server) before virtual scroll needs it like ionic infinite scroll .

May be exposing extra TableOptions like:
loadAmount: single load request amount.
distance: percentage indicator of scroll position when to load more data before it reaches vertical scroll.
maxLoadInCache: how many rows to have it in the cache.

Well these were my thoughts

@amcdnl
Copy link
Contributor

amcdnl commented Sep 13, 2016

Check out 0.5.1, I updated my API to return what you were expecting. I do not see that typo, you reference. Can you paste a link to the code in GH?

Yup, I understand infinite vs virtual. Well technically my thought is infinite is when you hit the bottom of the page and loads next and virtual is the whole pane is painted w/ the exact count. You are talking about lazy-loading data based on virtual pagination. Which as far as I know should be supported, I have this in my app so I'll make a demo to illustrate this.

I somewhat disagree with your point about body scroll not calling page because technically it is paging its just not from a traditional pager. It has to be triggered different though so the offset can be set if its triggered externally ( ie footer pager, outside api, etc ).

Regarding the API suggestions, some thoughts:

  • loadAmount: I don't see this as the decision of the table, you can ask your server for how much ever you need and load them. If you wanna load 2 pages at once ( i do in production ), it doesn't care.
  • distance: This is handled by offset. By loading more data you can have a check that says if that is loaded, no reason to go back.
  • maxLoadInCache: not entirely sure why you would need this.

@zxshinxz
Copy link
Author

I somewhat disagree with your point about body scroll not calling page because technically it is paging its just not from a traditional pager. It has to be triggered different though so the offset can be set if its triggered externally ( ie footer pager, outside api, etc ).

That explanation clicked my brain. Thank you for the explanation 😄

Can you paste a link to the code in GH?

I must have mistaken with different source. sorry for the confusion.

@amcdnl
Copy link
Contributor

amcdnl commented Sep 14, 2016

No worries! Cheers!

@shivagarwal08
Copy link

shivagarwal08 commented Sep 26, 2016

Hi All,

I need to fetch 20 records at a time via a service using observable, this service takes page_number and item_per_page=20 values, and returns the data for the given page_number.
Now I need to display these records in data-table. At a time table displays say 10 records, so when user scrolls down it should display next records and also call an event (say page event) which will again fetch the 20 records from server, and these should be added to data-table. I also want to remove few records from the top of the list which are not visible.
Like wise if i scroll up, it should fetch the records for previous page.

Please help me on this or if you are able to give a sample code that would be great.

Thanks for this wonderful data table. its very easy to implement and provides various functionalities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants