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

fix(mobile): asset state remain in gallery view after being deleted #10603

Merged
merged 4 commits into from
Jun 27, 2024

Conversation

alextran1502
Copy link
Contributor

@alextran1502 alextran1502 commented Jun 24, 2024

Fix #6401

Superceed #6966

@alextran1502
Copy link
Contributor Author

@martyfuhry, I refactored your PR #6966. One issue I encountered was that the app would crash when trying to precache the next asset index if we removed the last photo in an album. Would you have any thoughts on this?

@martyfuhry
Copy link
Contributor

Thank you! That old code may have had an issue or two, because it is mostly a hacky workaround. The ideal scenario is for both the gallery and the grid to watch and reference the same provider. Then everything would be in-sync between them. The provider would even know which index was the most recently viewed in the gallery, and the grid could scroll to that location when you return.

Deleted assets would update correctly in the gallery because the gallery would update its collection from the provider. In particular,

  1. The gallery tells the provider to delete an asset
  2. The provider deletes that asset and notifies the gallery of the change
  3. The gallery is rebuilt
  4. The new gallery references a new RenderList which does not contain the deleted asset

The main issue with this workaround is that we are passing in a RenderList into the gallery. This RenderList may become "out of sync" with the real RenderList because the gallery widget is Stateless and will not be notified of RenderList changes. My solution was to "mimic" the deletion in the RenderList which is cached in the gallery.

One issue I encountered was that the app would crash when trying to precache the next asset index if we removed the last photo in an album. Would you have any thoughts on this?

In this case, do we even need to rebuild the gallery after deleting the asset? Just pop the context right after you delete the asset. Don't even bother with updating the renderList or the totalAssets. Those are only local to the gallery, and they will not be needed. This way, we can avoid dealing with the boundary issue entirely. And nothing bad has ever come from ignoring obvious issues. In this case, this is workaround, so it's probably fine for now.

@alextran1502 alextran1502 merged commit d8175d8 into main Jun 27, 2024
22 of 23 checks passed
@alextran1502 alextran1502 deleted the fix/mobile/fix-6401 branch June 27, 2024 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] iOS/Android: Deleting asset from asset view does not remove view from navigation history
2 participants