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

Mobile,Desktop: Resolves #10206: Allow marking a plugin as mobile-only or desktop-only #10229

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Mar 29, 2024

Summary

This pull request:

  1. adds support for marking a plugin as mobile-only or desktop-only, and
  2. improves the mobile UI for incompatible plugins in search.

Resolves #10206.

Notes

Compatible platforms can be specified with the "platforms" manifest key. Valid platforms are desktop and mobile. For example, "platforms": ["desktop"] would mean that a plugin only supports desktop and not mobile. To support additional future platform values, Joplin clients ignore other platform types.

Because changing the format of app_min_version would break old versions of Joplin, the platforms key also allows specifying different minimum versions for each platform. For example, "platforms": ["desktop", "mobile>=3.0.5"] would use the existing app_min_version on desktop and 3.0.5 as the minimum version on mobile. Currently, additional versions specified through platforms must be greater than or equal to the existing app_min_version.

If platforms is not given, it is assumed that the plugin supports all platforms.

Screenshots

Incompatible plugins are moved to the end of the search results and are made partially transparent:
screenshot: plugin search results with incomaptible plugins

Clicking on the "incompatible" badge shows the reason for the incompatibility:
screenshot: Alert dialog says "this plugin doesn't support Joplin mobile"
screenshot: Alert dialog says "Please upgrade joplin to version 30.0.0 or newer to use this plugin"

On desktop, incompatible plugins are also sorted to the end. The incompatibility description text has also been changed:
screenshot: Plugin search results on desktop, last plugins are greyed out and have yellow warning text in their boxes -- warning text is similar to that for mobile (e.g. "this plugin doesn't support Joplin Desktop")

Manual testing

Although this pull request has automated tests, it can be tested manually with the following steps:

  1. Apply the following diff to add search results:
diff --git a/packages/lib/services/plugins/RepositoryApi.ts b/packages/lib/services/plugins/RepositoryApi.ts
index d91d4f74f..86477caf9 100644
--- a/packages/lib/services/plugins/RepositoryApi.ts
+++ b/packages/lib/services/plugins/RepositoryApi.ts
@@ -195,9 +195,44 @@ export default class RepositoryApi {
    public async search(query: string): Promise<PluginManifest[]> {
  	  query = query.toLowerCase().trim();
 
-		const manifests = await this.manifests();
+		const manifests = [...await this.manifests()];
  	  const output: PluginManifest[] = [];
 
+		manifests.push({
+			id: 'this.is.a.test.1',
+			name: 'This is a test',
+			app_min_version: '3.0.0',
+			platforms: ['desktop'],
+			manifest_version: 1,
+			version: '0.0.1',
+		}, {
+			id: 'this.is.a.test.2',
+			name: 'This is a test (2)',
+			app_min_version: '3.0.0',
+			platforms: ['mobile'],
+			manifest_version: 1,
+			version: '0.0.1',
+		}, {
+			id: 'this.is.a.test.3',
+			name: 'This is a test (3)',
+			app_min_version: '3.0.0',
+			platforms: ['mobile>=4'],
+			manifest_version: 1,
+			version: '0.0.1',
+		}, {
+			id: 'this.is.a.test.4',
+			name: 'This is a test (4)',
+			app_min_version: '3.0.0',
+			manifest_version: 1,
+			version: '0.0.1',
+		}, {
+			id: 'this.is.a.test.5',
+			name: 'This is a test (5)',
+			app_min_version: '30.0.0',
+			manifest_version: 1,
+			version: '0.0.1',
+		});
+
  	  for (const manifest of manifests) {
  		  if (this.isBlockedByInstallMode(manifest)) {
  			  continue;
  1. Search for test
  2. Verify that incompatible plugins are sorted to the end of the results
  3. (Mobile only) Verify that clicking on the "incompatible" badge displays an explanation.

This has been tested successfully on an Android 12 emulator (mobile) and on Ubuntu 23.10 (desktop).

packages/lib/services/plugins/utils/isCompatible/index.ts Outdated Show resolved Hide resolved
Comment on lines -454 to +480
if (!this.isCompatible(plugin.manifest.app_min_version)) {
throw new Error(`Plugin "${plugin.id}" was disabled because it requires Joplin version ${plugin.manifest.app_min_version} and current version is ${this.appVersion_}.`);
if (!this.isCompatible(plugin.manifest)) {
throw new Error(`Plugin "${plugin.id}" was disabled: ${this.describeIncompatibility(plugin.manifest)}`);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I like this change — previously, the error message included more information about the current Joplin version than it does now. This change does keep the logged and UI error messages consistent, though.

@@ -75,6 +82,7 @@ If no promo tile is provided, your plugin icon will be displayed instead.
"version": "1.0.0",
"author": "John Smith",
"app_min_version": "1.4",
"platforms": ["mobile>=3.0.3", "desktop"],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative to including minimum versions directly within the platform string could be to make this a dictionary instead. For example,

Suggested change
"platforms": ["mobile>=3.0.3", "desktop"],
"platforms": {"mobile": ">=3.0.3", "desktop": true},

The above may be more difficult to work with, however, particularly if most plugins will specify "platforms": ["desktop"].

How other apps handle this:

  • Obsidian lets plugins specify an isDesktopOnly property. Joplin will likely need the ability to specify different minimum versions for different platforms, however (reasoning: 1) different mobile and desktop versions correspond to the same commit and 2) plugin APIs may be added to different platforms at different times).
  • Expo uses a list of supported platforms, for example [ "android", "ios" ].

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative could be to add an app_min_version_mobile property.

@DarkFalc0n
Copy link
Contributor

Is there a check in place to not display the 'recommended' chip, in case it is not supported in mobile?

@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Apr 1, 2024

Is there a check in place to not display the 'recommended' chip, in case it is not supported in mobile?

Not currently — I think this should be added though. (Note to self: Apply related desktop change in different pull request). Edit: Added in dc43da7.

@laurent22 laurent22 merged commit f899c97 into laurent22:dev Apr 3, 2024
10 checks passed
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.

Plugin metadata: Allow marking a plugin as mobile or desktop-only
3 participants