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

Embeds in Twenty TwentyOne #3

Closed
mrjarbenne opened this issue Mar 9, 2021 · 12 comments
Closed

Embeds in Twenty TwentyOne #3

mrjarbenne opened this issue Mar 9, 2021 · 12 comments
Assignees

Comments

@mrjarbenne
Copy link
Member

I don't think this is directly related to our OneDrive plugin, but it's affecting the display of content from it.

See: https://sandbox.commons.hwdsb.on.ca/2021/01/test/

and: https://hwdsb.staging.boreal321.com/2021/03/09/498/

When our MEXP content, or a core/Custom HTML block is added to the post, they don't align within the tags of the theme:

2021-03-09_11-15-42

I've deactivated most of the plugins on the staging server to try to discern the conflict by it doesn't seem to help.

Beyond this, are there other issues that need testing before we unfurl the OneDrive plugin on the rest of the network?

How are the privacy settings configured on the OneDrive embeds? On the Google Drive plugin we have a check box to adjust the permissions: is there something similar needed for OneDrive (I suppose if you can't see the embeds I shared in the above two posts we might know better what's up).

@mrjarbenne
Copy link
Member Author

2021-03-09_11-20-05

@r-a-y
Copy link
Member

r-a-y commented Mar 12, 2021

When our MEXP content, or a core/Custom HTML block is added to the post, they don't align within the tags of the theme

The alignment issue is because of Gutenberg. They wrap a <figure> element around the embed contents. I've addressed this in all our custom embed plugins by also wrapping the <figure> element around our embeds.

How are the privacy settings configured on the OneDrive embeds?

For OneDrive, the picker JS will automatically provision the file as read-only and with the "Anyone with the link can view" capability.

For SharePoint, the privacy settings can be tweaked further. For the Commons, it looks like any sharing link expires within a year, so current files that are embedded into posts will need to be re-embedded after the sharing link expires. This looks like a setting that can be configured at the SharePoint level:

2021-03-11_173706

Here are the docs if we wanted to look into configuring that:
https://docs.microsoft.com/en-us/sharepoint/turn-external-sharing-on-or-off#advanced-settings-for-anyone-links

Also, it looks like sharing links are only valid for those authenticated to the Commons. So when you attempt to view the Sandbox and boreal posts above, those embeds will not be visible if you are not logged into SharePoint.

This is also a configurable option in SharePoint:
https://docs.microsoft.com/en-us/sharepoint/change-default-sharing-link

Beyond this, are there other issues that need testing before we unfurl the OneDrive plugin on the rest of the network?

#1 is still an issue. I'm thinking for non-embeddable types, we just simply display an icon of the file that links to the OneDrive item.

It's either that, or we automatically refresh the embed when a user visits the page and the embed needs to be refreshed. This will require asking for permission, similar to our Google Drive implementation on the "Settings > General" page. A problem with this approach is the sharing link expiry date as well.

@mrjarbenne
Copy link
Member Author

The alignment issue is because of Gutenberg. They wrap a

element around the embed contents. I've addressed this in all our custom embed plugins by also wrapping the element around our embeds.

I'm seeing this issue even if I use the core Custom HTML option to insert a iframe-based embed code, not just our MEXP plugins. See here: https://sandbox.commons.hwdsb.on.ca/2021/03/embed-2/

#1 is still an issue. I'm thinking for non-embeddable types, we just simply display an icon of the file that links to the OneDrive item.

I'm with you here. An icon with a link sounds less taxing on the server than an automated refresh, especially if users have a ton of posts appearing in an archive, or embed multiple items on a post.

@r-a-y
Copy link
Member

r-a-y commented Mar 12, 2021

I'm seeing this issue even if I use the core Custom HTML option to insert a iframe-based embed code, not just our MEXP plugins. See here: https://sandbox.commons.hwdsb.on.ca/2021/03/embed-2/

That's a problem with the Twenty Twenty One theme. I would submit an issue to the Twenty Twenty one theme devs to see if they can fix it. I could also write a code snippet to automatically add a <figure> element for those iframes that do not have it as well.


Also let me know what you think about the default privacy settings. Are we okay with 1-year embeds and having embeds only viewable for those logged in to SharePoint? Or do we want to change any settings from the SharePoint admin side?

@mrjarbenne
Copy link
Member Author

That's a problem with the Twenty Twenty One theme.

I thought it might be but I was second-guessing myself

https://wordpress.org/support/topic/custom-html-block-missing-div-on-front-end/

Also let me know what you think about the default privacy settings.

I think at this point we need to run with it as-is. I've requested a change to that setting but that will be a longer discussion in IT.

@mrjarbenne
Copy link
Member Author

mrjarbenne commented Mar 12, 2021

I could also write a code snippet to automatically add a <figure> element for those iframes that do not have it as well.

It seems to be contained to 2021. Let's see if they offer a fix.

@mrjarbenne
Copy link
Member Author

mrjarbenne commented Mar 16, 2021

I could also write a code snippet to automatically add a <figure> element for those iframes that do not have it as well.

@r-a-y we might need that snippet after all. I'm not winning the hearts of the WP devs with my argument: https://core.trac.wordpress.org/ticket/52828

@r-a-y
Copy link
Member

r-a-y commented Mar 16, 2021

Can you tell me which themes require the fix? I seem to remember you saying that other Twenty themes were affected, but I might be wrong.

@r-a-y
Copy link
Member

r-a-y commented Mar 16, 2021

I've added a snippet to /wp-content/mu-plugins/frontend.php, which adds the missing <figure> element for iframes that do not have it only for the Twenty Twenty One theme.

It's not perfect, but it does the job.

@mrjarbenne
Copy link
Member Author

It was just 2021. The others behave as expected. I'm going to keep pushing on the trac ticket, but it seems like an uphill battle. Maybe the custom html block is about to pivot to playing a key role in front end editing, and that's why they want it to behave without additional divs.

If that's the case they need a separate block called "embed codes" that behaves within the entry-content div.

@mrjarbenne
Copy link
Member Author

@r-a-y Can you take a look at your fix again for me. It looks like if there are multiple embeds in a post, the last one doesn't get corrected:

https://sandbox.commons.hwdsb.on.ca/2021/03/embed-test/

@r-a-y
Copy link
Member

r-a-y commented May 4, 2021

Can you take a look at your fix again for me. It looks like if there are multiple embeds in a post, the last one doesn't get corrected:

Fixed.

My regex replacement checks for two line returns after the </iframe> before wrapping the <figure> around it to prevent conflicts with other code.

The Jetpack ShareDaddy code runs at the end of the post content and conflicted with this because it only has one line return before adding their code. Thus, the replacement did not occur. So what I did was add a filter to the JetPack ShareDaddy display to add an extra line return. Now, the <figure> element should wrap around the last <iframe> in the post as well.

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