Skip to content

Compatiblity: ensure compatibility with Jetpack 11.8 - #7

Open
jeherve wants to merge 2 commits into
janboddez:mainfrom
jeherve:update/mastodon-rm-in-jetpack
Open

jeherve wants to merge 2 commits into
janboddez:mainfrom
jeherve:update/mastodon-rm-in-jetpack

Conversation

@jeherve

@jeherve jeherve commented Feb 1, 2023

Copy link
Copy Markdown

Nice plugin! As Jetpack has made some changes in its upcoming version (scheduled to be released next Tuesday), I wanted to reach out and suggest a change to ensure full compatibility between the 2 plugins.


In Jetpack 11.8 or higher, the plugin includes Mastodon as a valid Social Menu option. Let's fallback to Jetpack's implementation in this case.

This only fallbacks to Jetpack for the Social Menu logic; it doesnt touch the block theme feature, since that's not supported by Jetpack.

For reference, here is Jetpack's PR:
Automattic/jetpack#28175

In Jetpack 11.8 or higher, the plugin includes Mastodon as a valid Social Menu option. Let's fallback to Jetpack's implementation in this case.

This only fallbacks to Jetpack for the Social Menu logic; it doesnt touch the block theme feature, since that's not supported by Jetpack.

For reference, here is Jetpack's PR:
Automattic/jetpack#28175
@janboddez

janboddez commented Feb 2, 2023 •

Copy link
Copy Markdown
Owner

Just a question: is there a way to "force" a Mastodon icon based on the "label," like I do in this plugin? Like, if the label says "Mastodon," maybe Jetpack can assume Mastodon rather than rely on a (limited) list of instances? Seems like a safe enough bet. (Maybe in a next Jetpack version?)

I noticed in Jetpack there is a way to filter the instance list, yet whoever happens to be using this plugin (I have no idea if that's going to be a lot of people or not, since newer "default" themes have their own icon functions and the Site Editor has its Social Icons block [which also has a Mastodon icon, by the way]) may very well see the icon "break" until they implement the correct filter callback.

@jeherve

jeherve commented Feb 2, 2023

Copy link
Copy Markdown
Author

is there a way to "force" a Mastodon icon based on the "label," like I do in this plugin?

There is not, that's a good point. You can disregard this PR then!

It's something we had avoided doing when we first implemented Social Menus, to avoid potential mixups. I don't think we would come back to that in Jetpack now, especially since we're not really actively developing Social Menus anymore, for the reason you mentioned: it's very true that less and less people are going to access the Social Menu feature :)

whoever happens to be using this plugin (I have no idea if that's going to be a lot of people or not, since newer "default" themes have their own icon functions and the Site Editor has its Social Icons block [which also has a Mastodon icon, by the way]) may very well see the icon "break" until they implement the correct filter callback.

Yup, folks would have to know about the filter indeed, or will get to know about it only when contacting support or googling.

@janboddez

Copy link
Copy Markdown
Owner

Was thinking (not sure of the added value, but okay) we can check against the instance list or something, to decide which way to go.

There may be another conflict, though, and that's that there would be two SVG symbols with id="icon-mastodon". Not sure which one browsers would pick, presumably the first one, but it'd still "invalidate" the HTML. Anyway, that I can fix with a prefix.

@janboddez janboddez mentioned this pull request Mar 1, 2023
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

Successfully merging this pull request may close these issues.

2 participants