search

decaporg / decap-cms

A Git-based CMS for Static Site Generators
https://decapcms.org
MIT License
17.87k stars 3.04k forks source link

Posts not fully loaded when using the back button #1744

Closed mryanb closed 5 years ago

mryanb commented 6 years ago

Describe the bug When you visit a post and press the back button, it doesn't load all of the posts that were previously there.

To Reproduce

  1. Go to list of posts
  2. Click on a post
  3. Click the back button in the browser
  4. See error

Expected behavior Expecting to see all the posts loaded

Screenshots

2018-09-13_09-43-43

Applicable Versions:

erquhart commented 6 years ago

@Benaiah pagination thang?

Benaiah commented 6 years ago

@erquhart my guess is that this is most likely another instance of the react waypoint we use to load more posts on scroll not firing its visibility event despite the fact that it's visible. Perhaps we should add a "load more" button that manually triggers pagination so users can manually fire it when the waypoint fails.

What I think is happening:

There's a variety of ways we can work around the bug, but the best way forward is likely to decouple API pagination from UI pagination, fix the instances where waypoint isn't indicating we need more entries, and add a manual fallback to load more entries just in case. The pagination already needs some UI work to indicate that it's loading more pages, so the manual trigger could probably be as simple as clicking the loading indicator.

erquhart commented 5 years ago

Is the react-waypoint bug documented in their repo? I'd almost rather see us submit a fix there than introduce a button here.

LoicMahieu commented 5 years ago

I found a similar bug:

kapture 2018-12-06 at 16 20 17

The problem here is that react-waypoint has already been mounted once, and this instance is reused across collections. react-waypoint trigger the onEnter, when initially visible, only at mount.

erquhart commented 5 years ago

Sounds like we need a PR that repositions the waypoint so that it's remounted when a new collection is selected.

sondrele commented 5 years ago

@erquhart I'm experiencing the same issue.

I tried passing a child to the Waypoint component, and this change triggered the collection to re-fetch entries like the first time it's opened. Could we utilize this to show e.g. a loading indicator in the Waypoint component? I'm not sure whether that's considered a hack or not, but it seams to be a feature of Waypoint component and it does seem to have the desired behavior.

On a different, but somewhat related, note - are the collections supposed to be re-fetched when moving between them in the UI (like the gif above)? It makes the UI jitter a little bit.

erquhart commented 5 years ago

Not sure why it's completely refetching when you pass Waypoint a child, but we shouldn't need to do that. I'd expect to render a new Waypoint when we render a collection, whether the collection was previously navigated to or not. Superfluous fetching should be avoided at a higher level - and I don't see any in the gif above, it only shows fresh fetching (with the loading indicator) when a collection is first loaded. No spinner when navigating back to a collection that was previously loaded.

sondrele commented 5 years ago

..., it only shows fresh fetching (with the loading indicator) when a collection is first loaded. No spinner when navigating back to a collection that was previously loaded.

Maybe I'm mistaken, but this is what I see in the gif above:

  1. Reload page
  2. Select the Pages collection
  3. Go back to the Translations collection
  4. Observe that all previously fetched translations are visible for ~0.5s before they get re-fetched (and then we encounter the bug reported in this issue).
barthc commented 5 years ago

@sondrele is right there is a re-fetch happening. This is where the re-fetch happens https://github.com/netlify/netlify-cms/blob/master/packages/netlify-cms-core/src/components/Collection/Entries/EntriesCollection.js#L37 So using the gif for example, when the user hits the translations entry page the second time, the loadEntries that is called would replace this collections ids here https://github.com/netlify/netlify-cms/blob/master/packages/netlify-cms-core/src/reducers/entries.js#L54 instead of appending because it's not a pagination call, and those ids is where the entries from the collection list pages reads data from https://github.com/netlify/netlify-cms/blob/master/packages/netlify-cms-core/src/components/Collection/Entries/EntriesCollection.js#L70. It's also explains the brief ~0.5 flash. I also don't think the react-waypoint is the culprit here.

I will send in a PR to fix the issue shortly, also thanks @sondrele for pointing out the re-fetch by just looking at the gif :+1:

barthc commented 5 years ago

@LoicMahieu we now have a PR to fix your related issue.

@erquhart I believe this issue is still related to unnecessary loadEntries calls, which replaces the already loaded paginated id entries. Here is another gif that duplicates the issue.

anim

Notice after hitting the editor screen and back to the collection list page, the already loaded paginated entries are gone and you would need to scroll down for them to load again. PR #1881 and #1964 should fixed the issue.

erquhart commented 5 years ago

Merged both, will release soon.