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

Pagination intermittently doesn't work #40

Open
francisbyrne opened this issue Jun 16, 2014 · 19 comments
Open

Pagination intermittently doesn't work #40

francisbyrne opened this issue Jun 16, 2014 · 19 comments

Comments

@francisbyrne
Copy link
Contributor

I've noticed that the pagination for Collections-based datatables just doesn't work sometimes. It seems to happen when you navigate away from a page with a datatable on it (via iron-router) then navigate back to the original page and use the pagination.

To Reproduce:

  1. Clone https://github.com/francisbyrne/finance
  2. mrt install; meteor
  3. Go to localhost:3000 in browser
  4. Sign up a new account
  5. Open console and enter loadDummyImports() (that will load a bunch of trades to populate the table)
  6. Test the pagination; it should work
  7. Navigate to "Trades" then back to "Portfolio"
  8. Test the pagination; this time it doesn't work

I'll try and reproduce it with a simpler example but here are some screenshots from my finance project in the meantime.

screen shot 2014-06-16 at 11 34 12 am

screen shot 2014-06-16 at 11 34 21 am

@serenitus
Copy link

Hi Francis,

It's possible, looking at your code, that you've hit the same problem I hope I've just solved. You have structured things the same way I had, by implementing the DataTables configuration as a template helper. After going through a whole evening's worth of trial and error contrasting what I had against the example, I put the config in the data property of the IronRouter controller thus:

SearchController = RouteController.extend({
    template: 'noticesSearch',
    data: function () {
        'use strict';

        return {
            rows: {
                columns: [{
                    title: 'Date',
                    data: 'noticedate',
                    mRender: function (data, type, row) {
                        return moment(row.noticedate).format('DD/MM/YYYY');
                    }
                }, {
                    title: 'Died on',
                    data: 'diedon',
                    mRender: function (data, type, row) {
                        return moment(row.diedon).format('DD/MM/YYYY');
                    }
                }, {
                    title: 'Family name',
                    data: 'familyname'
                }, {
                    title: 'First names',
                    data: 'firstnames'
                }, {
                    title: 'Estate value',
                    data: 'estatevalue',
                    mRender: function (data, type, row) {
                        return accounting.formatMoney(row.estatevalue, '£ ', 0);;
                    }
                }],
                subscription: 'all_notices',
                options: {
                    order: [0, 'asc']
                }
            }
        };
    }
});

While this has fixed the problem I'd be interested to hear Austin's thoughts. Especially as the README.md shows exactly the "broken" technique.

Hope this helps!

@therealyoram
Copy link

I encountered the same problem today, and after spending hours and hours… i've found a solution: change the "id" to an random value like Meteor.uuid().

pull request: https://github.com/francisbyrne/finance/pull/79

my bug reproduction process:

go to http://localhost:3000/
go to http://localhost:3000/transactions
go to page 6
goto http://localhost:3000/
big bada boom: pagination is dead

@serenitus
Copy link

@joramhoefs that is one strange fix. It's quite hard to comprehend how that would make a difference as afaik that value is just used to provide a DOM element id to assist in applying CSS, etc. For this reason using a UUID is certainly counter to this purpose of this property!

@francisbyrne
Copy link
Contributor Author

Thanks so much @joramhoefs!

It fixes my problem so I've merged the pull request but I think I'll leave this issue open for @austinrivas to respond to because it seems like a weird fix and maybe masking an underlying issue as @serenitus mentioned.

@therealyoram
Copy link

yes, it's not a clean solution. rather a dirty workaround with a slightly sarcastic note.
all hail our random non-identifying overlords! ;)
but it works with all the negative side effects. i'll deal with the real problem-as soon as i've time …

@serenitus
Copy link

I wonder if there's a problem whereby the rendering stage is finding that a DOM element with the specified id already exists. That would perhaps explain why your change works.

@francisbyrne
Copy link
Contributor Author

@joramhoefs Weirdly enough, your fix with Meteor.uuid() actually breaks the reactive query I have with Session variables . See https://github.com/francisbyrne/finance/issues/81 for reproduction.

It appears we are opening a can of worms for this package! 😟

@francisbyrne
Copy link
Contributor Author

Ah, I see what is happening. Every time it re-renders the table after the reactive query changes, it uses the original selector to get the table DOM but, since that has changed to a new uuid, it doesn't find anything.

Two solutions:

  1. Change this.selector() to lookup the selector value at every time it runs, instead of storing a static data.selector value. Or,
  2. Document that you can't use changing values (i.e. RNG values) as selectors, and fix the underlying pagination issue.

@austinrivas What do you think?

@serenitus
Copy link

@francisbyrne 1) sort of defeats the purpose of the id. You might as well just look up the class instead. Kills any potential for having multiple tables on the page also. 2) That would be the correct thing to do IMO.

There's 3) if you're using IronRouter, switch to the mechanism I described in my first response.

@chadrik
Copy link
Contributor

chadrik commented Jun 22, 2014

Hey folks, @austinrivas has been sick the last week which is why it's been so quiet around here. He should be back to work sometime next week, I think. Just wanted to give you all a heads up.

@francisbyrne
Copy link
Contributor Author

@chadrik thanks, good to know.
@serenitus Okay, I'm trying to switch to the Iron-Router solution you suggested but I keep getting errors. With your code block above, do you just call {{> DataTable}} in the template?

@serenitus
Copy link

@francisbyrne for me it looks like:

{{> DataTable columns=rows.columns subscription=rows.subscription options=rows.options }}

@francisbyrne
Copy link
Contributor Author

@serenitus Hmmm, I can't seem to get it working with your approach.

Firstly, the reactive query doesn't work with this solution it seems; I just keep getting Exception from Deps recompute function: Error: Can't call non-function: undefined

Secondly, even if I ignore the query, I still get the same pagination issues as before.

So I'm just going to switch back to having it in the helper and see what @austinrivas comes back with regarding the pagination issues.

@serenitus
Copy link

@francisbyrne So I now have a reactive query working also using my technique. I see the recompute error occasionally though so it looks like there's a race somewhere. I'll try and investigate tonight.

@serenitus
Copy link

@francisbyrne looks like the race issue was a correct guess. I moved my initialization of the reactive query Session.set('reactive-query', {}); from IronRouter's onBeforeAction into the main body of my js file.

That has done the trick.

[EDIT] Spoke too soon. That worked for the first filter only! Subsequent filter entries did nothing. sigh

@jasonxeno
Copy link

Has anyone had any luck battling with this lately? BTW not sure if it is any help but this seems related to be the same issue I documented here also: #38

I feel like there could be a hack that just erases everything connected to table when the template is destroyed but I haven't had any luck.

@gjolund
Copy link
Contributor

gjolund commented Jul 1, 2014

@jcswanson the core issue is that meteor does not preserve the template context on reactive rerenders, which destroys the datatable instance that is bound to the template component.

Once the meteor team adds an updated callback as they have been discussing that provides access to the component context this issue will be resolved.

I'm hesitant to apply any more work arounds until the updated api starts taking shape.

@jamesdwilson
Copy link

Hi @austinrivas, I really appreciate your hard work on this component!

Is it possible for us to hack in an updated callback until then?

I really want take advantage of all your hard work, this looks way better than reactive tables.

Do you have an MDG bug ID for this issue?

Thank you

@gjolund
Copy link
Contributor

gjolund commented Jul 2, 2014

It's not technically a bug so much as a needed feature, there is an entry on the meteor trello roadmap that identifies the issue. Please bump it if you have the time https://trello.com/c/bv3pmX2x/51-fancy-animation-support & https://trello.com/c/Jg9N9F1l/62-new-object-oriented-api-for-ui-components

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

No branches or pull requests

7 participants