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

Future risks for this library #26

Open
johnbillion opened this issue Apr 9, 2021 · 4 comments
Open

Future risks for this library #26

johnbillion opened this issue Apr 9, 2021 · 4 comments

Comments

@johnbillion
Copy link
Member

johnbillion commented Apr 9, 2021

The main reason AMF can work as intended is that the media library in WordPress gets its attachments populated via an admin-ajax.php request which uses a response format which is not a representation of WP_Post objects (see the wp_prepare_attachment_for_js() function). This allows AMF to override what gets returned and displayed in the media library without an attachment having to exist for each media file.

If this were to change in the future then AMF would cease to work. With that in mind I've collected some risks and potential mitigations.

The admin-ajax.php attachment requests in the media manager get replaced

If the media modal or media grid get rebuilt using the REST API (or GraphQL) instead of the existing admin-ajax.php endpoint to fetch attachments then a real attachment would need to exist for each media item, because that's the response format that's expected.

To mitigate this it might be possible to override requests to the REST API for media when browsing the media manager in order to use a different endpoint which returns placeholder IDs for each "attachment". Not ideal, this sends us down the route of lying about attachment IDs which is what this library intended to get us away from.

The Backbone JS for the media manager gets replaced

If the media modal or media grid get rebuilt in React (but continue to use the existing Ajax endpoint for fetching attachments) this would break the extend_toolbar() function which overrides the behaviour of the "Insert" or "Select" buttons in the Backbone view that triggers the creation of an attachment at the point where a media file gets used.

To mitigate this we'd need to ensure AMF can still hook into the point where a media file gets "used" so it can be intercepted and an attachment created just as it does now.

Both of the above

A double hit, and to be honest if one happens it's likely the other will too.

@johnbillion
Copy link
Member Author

The infinite scrolling removal in WP 5.8 has broken AMF because the response format of the Ajax call has changed, although compatibility can be added. See https://core.trac.wordpress.org/ticket/50105 at comment 81 onwards.

@johnbillion
Copy link
Member Author

Media library work has started as part of Gutenberg phase 3: WordPress/gutenberg#55238

@tomjn
Copy link
Contributor

tomjn commented Jul 6, 2024

If the media modal or media grid get rebuilt in React (but continue to use the existing Ajax endpoint for fetching attachments) this would break the extend_toolbar() function which overrides the behaviour of the "Insert" or "Select" buttons in the Backbone view that triggers the creation of an attachment at the point where a media file gets used.

WP.com already replaced the media library in Calypso with a react implementation years ago so there's already prior art to draw on for gutenberg work if it's a straight up replacement of the modal

Screenshot 2024-07-06 at 19 00 01

Of note, this is in the standard block editor, they've yanked the media modal out and put a new one in. It's possible they maintain a fork of Gutenberg in order to do this but it might be feasible to do this ourselves if the cost is worth it.

If the media modal or media grid get rebuilt using the REST API (or GraphQL) instead of the existing admin-ajax.php endpoint to fetch attachments then a real attachment would need to exist for each media item, because that's the response format that's expected.

To mitigate this it might be possible to override requests to the REST API for media when browsing the media manager in order to use a different endpoint which returns placeholder IDs for each "attachment". Not ideal, this sends us down the route of lying about attachment IDs which is what this library intended to get us away from.

This has sort of half happened already

Screenshot 2024-07-06 at 19 01 33

There's an API tied to this that could remove the media library as a source and insert new providers, and already does so for OpenVerse out of the box.

Media library work has started as part of Gutenberg phase 3: WordPress/gutenberg#55238

This looks like it's intended to replace the media admin menu using adjustments to standard data views and a new attachment edit screen, no designs for the backbone modal pop up yet though

@tomjn
Copy link
Contributor

tomjn commented Jul 6, 2024

This is how the wp.com react media modal implemented multiple sources:

Screenshot 2024-07-06 at 19 04 54

Though google images is missing from the inserter implying they have separate sources:

Screenshot 2024-07-06 at 19 05 44

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants