-
Notifications
You must be signed in to change notification settings - Fork 79
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 Save fetched files so details button works after pagination #1221
FIX Save fetched files so details button works after pagination #1221
Conversation
8c7dd70
to
a34f4cf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's super dodgy and won't work if you try accessing a file directly by its link.
Is it? It's just caching a result
Not sure I follow. Do you mean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's already caching built into AssetAdmin. That's why you don't get a GraphQL request when you navigate to a folder you've already visited. If we wanted a way to recall results of a previous page, we should try to interact with that GraphQL caching directly, rather than subvert it in AssetAdmin.
The fundamental problem is not caching. The problem is that the the Editor form is missing bits of data that it expects to be passed to it by AssetAdmin:
- Let's say you have selected a file that is on the second page and you saved your DataObject
- You refresh you CMS screen (your
fetchedFiles
cache will be empty at that point) - You click the view icon on the UploadField
- The AssetAdmin modal popup and load the first page of results ... that page won't contain your selected image because it's on the second page. It's not in the cache either because you never accessed the second page of results.
I tried running your PR locally and that's exactly the behaviour I saw.
There's detailed explanation of the problem and how I think it should be approached on the issue.
@@ -60,6 +64,14 @@ class AssetAdmin extends Component { | |||
} | |||
} | |||
|
|||
componentDidUpdate(prevProps) { | |||
// Save file data in case we need to use it later after paginating the gallery | |||
prevProps.files.forEach(file => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Array.reduce
would be better here
Not that it matters.
Issue #1136