Added favicon size to google provider#13
Added favicon size to google provider#13jayenne wants to merge 6 commits intostefanbauer:masterfrom jayenne:add_favicon_size_to_google_provider
Conversation
Added favicon_size to config file
stefanbauer
left a comment
stefanbauer
left a comment
There was a problem hiding this comment.
I merged a DuckDuck Provider in the meantime. I think it makes sense to add those favicon sizes to all providers as well.
| /* | ||
| |-------------------------------------------------------------------------- | ||
| | Favicon Provider | ||
| |-------------------------------------------------------------------------- | ||
| | | ||
| | This value is used for requesting a favicon of the given size (if supported by the given provider) | ||
| | | ||
| */ | ||
|
|
||
| 'favicon_size' => 32, | ||
|
|
||
| /* | ||
| |-------------------------------------------------------------------------- | ||
| | Favicon Provider | ||
| |-------------------------------------------------------------------------- | ||
| | | ||
| | This value is used for requesting a favicon of the given size (if supported by the given provider) | ||
| | | ||
| */ | ||
|
|
||
| 'disk' => 'public', | ||
|
|
There was a problem hiding this comment.
I think it makes sense to put those values below the provider_class configuration with a simple description. It looks weird to me to have duplicate "favicon provider" headlines. The description above the disk is also wrong cause it's just copied from the favicon size.
src/Provider/GoogleProvider.php
Outdated
| $favicon_size = config('favicon-extractor.favicon_size', 32); | ||
| $size_query = '&sz='.$favicon_size; |
There was a problem hiding this comment.
Variables please in camelCase faviconSize, sizeQuery
Moved config variables between provider and generator
unavatar is interesting because it resolves a domain, username or email to image.
stefanbauer
left a comment
stefanbauer
left a comment
There was a problem hiding this comment.
I think it's good. Just those little three modifications and I'll merge it 👍🏻
| private function getUrl(string $url): string | ||
| { | ||
| $faviconSize = config('favicon-extractor.favicon_size', 32); | ||
| $sizeQuery = $faviconSize.'/'; |
There was a problem hiding this comment.
Missing line-break before return
| { | ||
| return 'https://www.google.com/s2/favicons?domain='.urlencode($url); | ||
| $faviconSize = config('favicon-extractor.favicon_size', 32); | ||
| $sizeQuery = '&sz='.$faviconSize; |
There was a problem hiding this comment.
Missing line-break before return
| // size options are limited to 16 & 32 | ||
| $faviconSize = config('favicon-extractor.favicon_size', 32) > 16 ? 32 : 16; | ||
| $sizeQuery = '?size='.$faviconSize; | ||
| return 'https://favicon.yandex.net/favicon/'.urlencode($url).$sizeQuery; |
There was a problem hiding this comment.
Missing line-break before return
|
Sorry for the delay, I missed somehow this PR. |
2 participants
Added favicon_size to config file