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

Responsive extra with Turbolinks and pagy_array #115

Closed
alec-implicitdesign opened this issue Jan 10, 2019 · 12 comments
Closed

Responsive extra with Turbolinks and pagy_array #115

alec-implicitdesign opened this issue Jan 10, 2019 · 12 comments
Labels

Comments

@alec-implicitdesign
Copy link

Please refer to this. The same error is popping up again now using turbolinks and pagy_array.

It also looks like the code that fixed this issue originally has been removed, so I take it another solution was found?

@ddnexus
Copy link
Owner

ddnexus commented Jan 11, 2019

If I open a new empty tab, then go to a pagy url pasted in the address bar, it generates the error. After that if the tab is the same, it works as expected without errors.

The fix for this problem is still there untouched, so I suspect that in this particular case of the new tab, it was there even after the fix. Could you check that?

@ddnexus
Copy link
Owner

ddnexus commented Jan 11, 2019

I just found the problem... thinking about how to refactor it...

ddnexus added a commit that referenced this issue Jan 13, 2019
- moved handling of window event listeners into the responsive function
- added resize delay to Pagy.responsive
@ddnexus
Copy link
Owner

ddnexus commented Jan 13, 2019

The old #64 was not actually fixing the root cause, just mitgating it. Now cef823c should have fixed it. The fix is part of a few other improvements for javascript and AJAX that will go into the next release,

@ddnexus ddnexus closed this as completed Jan 13, 2019
@ddnexus ddnexus added fixed and removed WIP bug labels Jan 13, 2019
@alec-implicitdesign
Copy link
Author

As far as I understand this fix should be in 1.3? I'm still getting errors:

Uncaught TypeError: Cannot read property 'forEach' of undefined

@ddnexus
Copy link
Owner

ddnexus commented Jan 22, 2019

I cannot reproduce that. Could you share a repo that produce that error?

@ddnexus ddnexus reopened this Jan 22, 2019
@ddnexus
Copy link
Owner

ddnexus commented Jan 22, 2019

BTW, I assume the javascript served in your page is the last one, not cached somehow...

@alec-implicitdesign
Copy link
Author

The error actually just happens intermittently, but even when the error doesn't show up I still need to trigger a window resize event before the pagination shows up. This seems specific to the responsive pagination.

It's definitely not a cache issue as I just cloned it to a different machine. I can't share the repo I'm afraid, I'll have to try to setup a mini rails test project. If you know of any easy way to do this please let me know.

@ddnexus
Copy link
Owner

ddnexus commented Jan 22, 2019

Just create a new app with a pagy gem and initializer, plus the javascript part. You don't even need a db: you just set the :count in the controller action and render just the @pagy in the view. No need to list @records.

Something like:

  def responsive_test
    @pagy, _records = pagy(count: 1000)
  end
<%== pagy_bootstrap_responsive_nav(@pagy) %>

Or whatever responsive flavor you are using...

@ddnexus
Copy link
Owner

ddnexus commented Jan 22, 2019

OK, no need to create any repo. I think I understood the problem. It depends on how much stuff is in the page. Please try the dev branch.

@alec-implicitdesign
Copy link
Author

That seems to do the trick. For myself it doesn't really matter how many items I'm rendering (in an isolated case I was simply rendering a list [1..100] with page size of 10 and it still happened).

setTimeout doesn't feel like a great long term solution but I appreciate you getting to it so quickly.

@ddnexus
Copy link
Owner

ddnexus commented Jan 23, 2019

@alec-implicitdesign glad that it finally works.

With "stuff in the page" I wasn't referring to the count nor the items per page: I was referring to the elements in the HTML page, or better, how long the page takes to get displayed (not just "loaded"). The Pagy.responsive function calls the render function as soon as the responsive nav gets on the page (after document.load), but the actual container.clientWidth gets set only when everything in the page gets its own dimensions. That depends on the other elements on the page and I suspect that it depends also from the particular layout and specific elements and CSS rules, and might even be different from browser to browser.

I didn't find a better way to ensure that the container.cientWidth gets a proper value before calling render, than checking it periodically from the render itself, till the page is done with calculating the dimensions. That just postpones the execution of the rest of the function till when there is a value for the container.cientWidth. It works well and doesn't affect anything else, but if you have a better way to do it, I am open to improve it.

@ddnexus ddnexus closed this as completed Jan 23, 2019
@alec-implicitdesign
Copy link
Author

Okay thanks for clarifying. I will have a look at it when I get a moment and see if I can't find a way around a setTimeout.

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

No branches or pull requests

2 participants