-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Document how to set an extension image #30459
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The priority is reversed if that list I assume? Ie. If metadata is set that is used over the feedback above? "Logo" I would refrain from using as not every image/icon used will be an actual logo. I would suggest "icon" or "image" ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
Also, it's expected to be an external url not a reference inside the extension itself, correct? |
Ah good catch, probably should be |
I didn't do |
And yes, @maxandersen, the logic list was in ascending order of importance, so reverse priority. No, I don't know why I thought that was a helpful order to write it in. :) |
66a3c35
to
8cbaab6
Compare
8cbaab6
to
c3eb6be
Compare
Oh, looking forward to add the nice icon created by @insectengine for Quarkus GitHub App :). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall good but I would suggest we give bit of more requirements for the image.
especially type and if png/jpeg allowed then some size recommendations.
We don't really have any size requirements, since the framework resizes and crops for us, but I'll make that clear. Cropping also behaves differently for GitHub social media previews and images specified in the yaml, so I'll explain that. This is probably too much to squish into a one-liner annotation, so I'll add a section. |
@holly-cummins is SVG allowed? Also, while it does some cropping, how does it handle high DPI (i.e. how to not end up with a blurred image on macOS)? |
I had to test SVG support, since I’m relying on the framework for image processing. By default, SVG doesn’t work, so I’ve added some extra implementation to support it. This only covers the case where the image is set in the extension yaml. If we’re getting the image from GitHub, they have their own rules, which exclude SVG. @gsmet, do you have a link explaining the high DPI issue? I'm included to assume the sharp plugin does the right thing, but I couldn't search properly without understanding the issue better. |
@gsmet @maxandersen do you have thoughts on adding a new section vs making the bullet point description of the field a whole paragraph? I feel like if I add more detail I'm getting to where it needs its own section, but looking at https://quarkus.io/guides/extension-metadata, the structure doesn't really have a place to put extra information. So I could just add more specifics to the bullet point. There's not much people can do wrong, especially now that svg is supported too, but I can see they might like some reassurance that they can't get it wrong. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can merge it now but I'll let @maxandersen approve.
Let's get this in. Thanks. |
🙈 The PR is closed and the preview is expired. |
Now that quarkusio/extensions#23 is implemented, we should document it. @maxandersen and @gastaldi , what do you think of the key I've chosen in the yaml? I can change it (and the implementation) if we want something else.
The image display logic is
extension.yaml
: we use thatI didn't want to burden the docs with all that information, but can if we think it adds something.