flarum / issue-archive

0 stars 0 forks source link

Improve front-end pagination of the Discussion List #162

Open dsevillamartin opened 3 years ago

dsevillamartin commented 3 years ago

Issue created from flarum/framework#1829 and flarum/framework#1820 (pts 2 & 4). flarum/framework#1820 in particular has a lot of useful discussion.

Problems:

Solutions:


Comments from the PR

Too much SEO voodoo that I'd like to investigate and read up upon first, after seeing flarum/framework#1820 and https://webmasters.googleblog.com/2014/02/infinite-scroll-search-friendly.html. - Franz https://github.com/flarum/core/pull/1829#issuecomment-529813635

Now that I've seen it, I am unsure this approach makes sense without a scroll handler. When you click multiple times on "Load previous" and "Load more", which page are you expected to be "on" (reflected by the URL parameter)? I am kind of thinking it would always be the earliest one that's loaded - because the top of the page would remain the same once you refresh the page. This would mean only "Load previous" would change the URL state. I think we can use the links from API responses instead of building URLs ourselves - that should simplify the state on the client. - Franz https://github.com/flarum/core/pull/1829#issuecomment-601662905

For consistency with scrolling inside a discussion, the page parameter in the URL should update whenever moving up or down. Refreshing shows (roughly) the same section (i.e. page) that you were looking at before the refresh. - Franz https://github.com/flarum/core/pull/1829#issuecomment-612223355

askvortsov1 commented 3 years ago

I took a look through the pagination util commit, and I don't think it's nearly as bad as what you said makes it out to be. There are definitely some non-trivial data ownership issues, but in the long term it's probably necessary.

I also wonder whether it could replace some of the pagination of PostStream... Although that uses a per-post URL scheme. Kinda related: https://github.com/flarum/core/issues/2397 (pagination could definitely set some canonical stuff?)

dsevillamartin commented 3 years ago

@askvortsov1 Thanks for looking through it.

I think it might be a good idea if someone else is able to focus on developing this & perhaps improving on my code (if we do want to go that route). Every time I try to tackle this issue it doesn't go well (regarding time and the end result 😄).

askvortsov1 commented 3 years ago

Consider making PostStream abstract, it could be a base for a pagination / infinite scroll system