Support multiple favicons - #35
deberlhe-jb wants to merge 4 commits into
Conversation
0xTim
left a comment
0xTim
left a comment
There was a problem hiding this comment.
Thanks for this! I think this is a good idea, but I want to avoid breaking the API for now. Could we add this as a new API and deprecate the old one? We can deprecate the favicon property and turn it just compute it based off the first item from the array. That should give us a clean migration
cf2c81f to
10920ae
Compare
10920ae to
9d7fcf9
Compare
|
@0xTim if think these changes are working, let me know what you think ! |
0xTim
left a comment
0xTim
left a comment
There was a problem hiding this comment.
Ok I think we're there almost. The last thing I would like to add is disfavoured overload annotations to the old APIs since they both have default arguments to ensure the compiler picks the right one
| self.path = path ?? type.defaultPath() | ||
| } | ||
|
|
||
| @available(*, deprecated, message: "This initializer guesses the type from a string and is therefore not as reliable as init(type:path:).") |
There was a problem hiding this comment.
@0xTim I'm not sure wether I should add the @_disfavoredOverload annotation here since it would pick SVG as the default format instead of guessing a possibly more appropriate one. What do you think ?
2 participants
All browsers handle different favicon formats and dimensions.
While modern browsers can handle SVG fine, older ones rely on PNG files with different sizes, or ICO files.
Kiln only support one favicon, so setting a nice SVG icon will prevent some browsers to display it.
With this Pull Request, it will be possible to add different icons to use the most efficient one at highest resolution when possible, with fallbacks otherwise.
This also allows adding an apple-touch-icon.
@0xTim : This is a proposition, but it introduces a breaking change in the Theme configuration API, as I replaced
favicon: Stringwithfavicons: [FavIcon]. We can discuss changes to make or instructions to write for migration if needed.