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

Customize admin favicon using admin.icon_image setting #4051

Merged
merged 4 commits into from
Jun 2, 2019

Conversation

olegts239
Copy link
Contributor

Adding possibility to change favicon using admin.icon_image setting

Copy link
Collaborator

@MrCrayon MrCrayon left a comment

Choose a reason for hiding this comment

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

I think in this case a one line solution might be enough:

-<link rel="shortcut icon" href="{{ voyager_asset('images/logo-icon.png') }}" type="image/x-icon">
+<link rel="shortcut icon" href="{{ Voyager::setting('admin.icon_image') ? Voyager::image(Voyager::setting('admin.icon_image')) : voyager_asset('images/logo-icon.png') }}" type="image/x-icon">

But if you want to keep the if else for clarity, to not repeat html, I would do something like this:

@php
if (Voyager::setting('admin.icon_image')) {
    $admin_favicon = Voyager::image(Voyager::setting('admin.icon_image'));
} else {
    $admin_favicon = voyager_asset('images/logo-icon.png');
}
@endphp
<link rel="shortcut icon" href="{{ $admin_favicon }}" type="image/x-icon">

@olegts239
Copy link
Contributor Author

@MrCrayon thanks for your comment, i used the same code in my commit as there for unified code styling

MrCrayon
MrCrayon previously approved these changes Apr 1, 2019
Copy link
Collaborator

@MrCrayon MrCrayon left a comment

Choose a reason for hiding this comment

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

@olegtsoy I saw about your choice for code styling, it makes sense.

Works for me.

@emptynick
Copy link
Collaborator

I'm not a hundred percent sure that this is compatible with
https://github.com/the-control-group/voyager/blob/317ce0f2014b08ea4f0f3ead0f973f1c952f642c/resources/views/login.blade.php#L50-L55
What happens if you upload a .ico file? Also, I think some browser don't directly accept a PNG as a favicon (IE for example)

@MrCrayon
Copy link
Collaborator

MrCrayon commented Apr 3, 2019

According to https://en.wikipedia.org/wiki/Favicon PNG and ICO are supported on all modern browsers, including IE11.
If someone is still using older IE versions not seeing bookmark icon might be the last of their problems 😆

@emptynick
Copy link
Collaborator

Its more about type="image/x-icon".
Uploading a PNG might (not sure) result in errors/notices in the console.
Can't we just change it to image/png and only use image/x-icon for the default .ico file?

resources/views/master.blade.php Outdated Show resolved Hide resolved
@emptynick emptynick merged commit a672c36 into thedevdojo:1.2 Jun 2, 2019
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.

4 participants