-
Notifications
You must be signed in to change notification settings - Fork 36
Unify icon contributions and clarify supported formats #450
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
base: main
Are you sure you want to change the base?
Changes from 9 commits
a6367b1
aca5aee
6b4d6eb
5c88538
d847757
7818385
ceed366
47f6a66
af10fdb
cdf1ef4
a2b6cf3
a672fdb
f7ad430
e386a81
bccece4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
|
|
||
| from npe2 import PluginManifest | ||
| from npe2.manifest import PackageMetadata | ||
| from npe2.manifest.contributions._icon import Icon | ||
| from npe2.manifest.schema import ENTRY_POINT | ||
|
|
||
| SAMPLE_PLUGIN_NAME = "my-plugin" | ||
|
|
@@ -182,7 +183,19 @@ def test_visibility(): | |
|
|
||
|
|
||
| def test_icon(): | ||
| PluginManifest(name="myplugin", icon="my_plugin:myicon.png") | ||
| pm = PluginManifest(name="myplugin", icon="my_plugin:myicon.png") | ||
| assert pm.icon == "my_plugin:myicon.png" | ||
| pm = PluginManifest(name="myplugin", icon="https://example.com/icon.png") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that we should allow remote icons in plugins. This is a big privacy issue.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, this is a really important point, it let's someone track things, like the 1px by 1px white image in your emails.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I agree. I was just trying to keep previous "behaviour" :) |
||
| assert pm.icon == "https://example.com/icon.png" | ||
| with pytest.raises(ValueError, match="not a valid icon URL"): | ||
| pm = PluginManifest(name="myplugin", icon="http://example.com/bad_icon.png") | ||
| pm = PluginManifest( | ||
| name="myplugin", | ||
| icon={"dark": "my_plugin:myicon.png", "light": "https://example.com/icon.png"}, | ||
| ) | ||
| assert isinstance(pm.icon, Icon) | ||
| assert pm.icon.dark == "my_plugin:myicon.png" | ||
| assert pm.icon.light == "https://example.com/icon.png" | ||
|
|
||
|
|
||
| def test_dotted_plugin_name(): | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.