Attachment details pane navigation not working after opening 81st (or greater) item and refreshing the browser

[ppetrov2c]

Setup:

• clean 5.8;
• twentytwentyone active;
• no plugins active.

Steps to reproduce:

• open media page;
• click on the 41st item in the list;
• notice the left arrow is enabled (which is to be expected, since there are previous items);
• refresh the page.

Notice the left arrow now - it's disabled.

Not sure if this is intended, but since the item isn't loaded (in the grid), there's no previous but there's only next, which points to the first item. Clicking the right arrow goes to the it and you can't go back to the previous item.

[antpb]

Thanks for reporting this @ppetrov2c ! I'm leaving a comment to note that in 5.8.1 the count per page was increased to 80 so in the case of this report, you'll be wanting to check the 81st result to reproduce what's being reported in this ticket.

Marking 5.9 as we investigate. :)

[antpb]

Noting with the changes that doubled the count loaded initially, the problem number will now be 81st or greater. :)

[joedolson]

This is directly caused by the fact that if you load more, then refresh the page, the collection is reset to the first 80 items, regardless of the additional items you've loaded. This doesn't stop a currently selected item from loading, but does break the item navigation, since the current item is effectively not in the collection.

There are a couple possible paths I can think of:

1) Implement a URL parameter that tracks the collection, so a refresh reloads everything previously loaded and the collection is inclusive. This would also be beneficial for introducing pagination.

2) Introduce an update to the previous item navigation so that it goes back to the last item in the current collection if the item is not itself in the current collection. Probably easier to implement, but I'm not sure it creates a particularly useful UI.

[sabernhardt]

Moving to 6.0 for now, though this might be appropriate in a minor release.

[costdev]

Per the discussion in the bug scrub, I'm adding needs-patch and also dev-feedback so that the suggestions Joe outlined in this comment can get some input from others.

[antpb]

Considering the time remaining in milestone and the workload of folks that can bring this across the line, this should move to 6.1. Happy to move back if anyone is able to help out on it! :D

[mdawaffe]

Replying to joedolson:

There are a couple possible paths I can think of:

1) Implement a URL parameter that tracks the collection, so a refresh reloads everything previously loaded and the collection is inclusive. This would also be beneficial for introducing pagination.

2) Introduce an update to the previous item navigation so that it goes back to the last item in the current collection if the item is not itself in the current collection. Probably easier to implement, but I'm not sure it creates a particularly useful UI.

We could also decide that left/right navigation does not makes sense for a media item unless that media item is loaded by clicking through to it from the list view. That is, if you visit, for example, http://localhost:8889/wp-admin/upload.php?item=6 directly (or if you reload the page when on that URL), we could not show the left/right navigation buttons at all.

[antpb]

considering the new recommended path forward, this needs time to have an agreed and developed fix. Moving this to 6.2 to allow it the time needed.

[antpb]

Joe and I have been discussing this one but think that it is likely best fit for 6.3 as any changes here have high risk of introducing issues.

[costdev]

Hi @antpb, is this likely to get a patch during the 6.3 cycle, or should we move it to Future Release until it has some more discussion and a patch?

[antpb]

Still having trouble solving this but it is related to some other issues I'm working on so I'll move this to 6.4 to keep it in view of Media.

[nicolefurlan]

@antpb just checking in on the status of this on since we're ~2 weeks from the 6.4 RC1. Do you think we'll be able to patch this ticket for 6.4?

[nicolefurlan]

@antpb pushing this one to 6.5 since it doesn't yet have a patch. Feel free to change it to Future Release if you think it needs more time/discussion.

[antpb]

Added a patch here. When the library index exceeds the count that is displayed on refresh, it returns an index value of -1. If we click the first item in the media library, the index value correctly shows 0 which then gets -1 in the hasPrevious check and sets hasPrevious to false. In the case of the out of range index, the -1 gets -1 and becomes -2 so we can safely check for an initial value of -1 and deduce that it is outside the range of initial load and does indeed have previous items.

Trying to think of cases where -1 will incorrectly assume previous.

[joedolson]

So, this semi-works. It depends a bit on what our expectations are. This does re-enable the navigation; but it navigates back one place in the visible collection, which isn't the same as the collection that the item was opened from.

For example, if you click on the 85th item, it would normally go back to the 84th item. If you do the reproduction steps with the patch in place, you are able to navigate, but it goes back to the 79th item.

[antpb]

Going to move this to 6.6 as we have identified new challenges that need a bit more time to explore. I'm working on a refactor of my approach that tracks a count of triggers on the load more button so that we can initialize the collection with this insight. More to come!

[antpb]

I have an in progress local copy for this patch but it is not yet complete. We should give this more time to land it earlier in the release cycle ideally for more testing.

Moving to 6.7