hwdsb / onedrive

A WordPress plugin to embed OneDrive items using the OneDrive block or [onedrive] shortcode.
0 stars 0 forks source link

Embeds in Twenty TwentyOne #3

Closed mrjarbenne closed 3 years ago

mrjarbenne commented 3 years ago

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 commented 3 years ago

2021-03-09_11-20-05

r-a-y commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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.