-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Feature Request: Beer Menus shortcode has a new embed processs #91423
Comments
Support References This comment is automatically generated. Please do not edit it.
|
Hi @msilbers, the old version of the embed isn't broken, and we would like customers to be able to use the new version of the embed on wordpress.com in addition to the old version. I think ideally the shortcode would branch which JavaScript/HTML is loaded based on whether the param was a bar_id or menu_id. Please let me know if you have any other questions. Thanks! |
@ericds, I would be happy to help with getting this done on the back end. Could you share a sample menu ID so I can test this out? For reference, I am trying to get this out by generating the following HTML when a <script src="https://www.beermenus.com/web-menus.js" type="text/javascript"></script>
<beermenus-web menu-id="1234"></beermenus-web> However, I don't have a valid menu ID to work with, so the menu load/fetch is failing. |
Thank you @daledupreez! You can use this menu id: 33 And here's an example embed code for that menu id:
Please let me know if you have any other questions. |
@ericds, I have the updates working in my dev environment, but the styling really doesn't look good on my test site, for both the older menu and for the newer one. Could you help confirm whether there are items we can fix on your side? Also, as a broader comment, it might be simpler to define the embed via the oEmbed standard. I am not sure if that's something you've considered -- users only need to paste the correct link for the embed to work, and you get a link rendered as a fallback if the embed isn't supported. That would then be something you could offer for many different services, and have much clearer control over how the embed works. Older shortcodeYou can see this rendered at https://ddptest.food.blog/2024/06/10/test-beermenus-shortcode/ Newer shortcodeThis isn't in production yet. I was testing with the test menu you mentioned above, menu ID 33, on the same link as above. In this case, it looks like the main issue stems from the inline CSS declaring |
Amazing, thanks @daledupreez! I just set the background color to #fff on our end for that menu id, so the newer shortcode should look good now. What are the next steps to get this in production? We haven't dug deep into oEmbed, but it seemed more designed to embed a specific object (e.g. photo, video, etc.) vs. more complex HTML and CSS like our menus. That said we're going to take a deeper look and may explore that in the future. Thanks again! |
For tracking purposes: internal change is in D151683-code. |
Thanks @daledupreez, does that mean it's live in production or will be soon? |
As a follow-up here, I deployed the changes to production a few minutes ago. I have also gone ahead and updated the examples in the shortcode documentation. @ericds, it still looks like there may be some better defaults for the embeds, especially in terms of backgrounds, but feel free to ping me with any follow-up issues you find. |
@daledupreez thank you for shipping this! Everything has been working great. I'll let you know if we run into any issues in the future. Customers set up the background color and other design settings on our end, and that's been working well so far. |
What
WPcom has the option to embed menus from Beer Menus, as shown here: https://wordpress.com/support/wordpress-editor/blocks/shortcode-block/#miscellaneous
This embed option is built in, but the developers are changing how it works and have reached out to us to get it updated
Why
This is an existing shortcode that would need an update. I'm specifically listing it as a feature request because I cannot yet tell if the old way is actually broken.
How
Quotes from the devs from this ticket: 8216240-zen
Some relevant details on p2: p58i-4gv-p2
The text was updated successfully, but these errors were encountered: