Make sure the Distributor admin bar works correctly when loading via AMP #665
Conversation
… This allows us to load the templates in the mustache format, which AMP better supports. Move our templates to a new templates directory, for cleaner code organization. Modify our push JS to account for this new template format
… mustache templates. Only show search bar if we have more than 5 connections. Change the name we show for each connection depending on if it is internal or external. Make sure disabled attribute is set correctly
|
Something looks off from Travis, haven't dug in yet to see what's erroring there but may need to if this fails testing with @rickalee. |
|
@dkotter Great work so far. Looks like we still need to address jQuery compatibility. Screenshot is AMP Validation errors. After I 'Kept' jQuery in AMP Validation, I was able to successfully push to External Location from frontend. |
@rickalee Thanks for the review! I do add the dev mode attribute to the jQuery script but am currently looking for a script with an id of The former was what I was seeing when testing so I'll look at this again and see if there's a more resilient way to look for jQuery that will work even if that ID changes (though I'm not sure why the ID is different, something else I can look at) |
…ake it less fragile if the IDs of those elements change
…ms WordPress uses that if you request just jquery. Make sure we don't load our mustache templates in the admin
|
@rickalee I've added some code that I think makes the handling of dependencies (in our case, jQuery and underscores) a bit more resilient. I couldn't ever reproduce a scenario that gave me a jQuery file with the ID of |
|
@dkotter Confirmed that Distributing is working on frontend and not getting any AMP Validation errors. Also confirmed jQuery isn't added to frontend for non-logged in users. I think we are set with this feature. However found another character encoding bug with apostrophe/single quote with titles but will open a different ticket. |
Description of the Change
When using the official AMP WP plugin, if you have the front-end of your site set to always use AMP and you're logged in, the Distributor admin bar item shows but does not work. The issue lies with AMP removing the assets we rely on, as well as mangling the templates we output.
This PR ensures all of our assets are loaded via amp-dev-mode, so they don't get removed. It also changes our template system too utilizing Mustache, which AMP supports.
Alternate Designs
None
Benefits
Front-end distribution will now work properly when using AMP
Possible Drawbacks
We now have two different template systems: one used only for AMP and the previous one we use elsewhere. Ideally we would use the same templates for both but for now, I think it's safer to keep those separate.
Verification Process
Checklist:
Applicable Issues
Closes #658