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

fix: add type to favicon field #147

Merged
merged 1 commit into from
Jan 3, 2024
Merged

Conversation

nobkd
Copy link
Collaborator

@nobkd nobkd commented Jan 2, 2024

resolve #146

Adds the image type to the favicon field

@@ -53,7 +54,7 @@ export function renderHead(data, is_prod) {
if (components) pushMeta('nue:components', components.map(uri => `${base}${uri}`).join(' '))

// misc
if (favicon) head.push(`<link rel="shortcut icon" src="${favicon}">`)
if (favicon) head.push(`<link rel="shortcut icon" type="${TYPES[extname(favicon).slice(1)]}" src="${favicon}">`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe should to lower case here? otherwise, .PNG doesn't work

Copy link
Contributor

Choose a reason for hiding this comment

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

Nicely spotted TYPES in nueserver.js! I think it's good to merge like this for now. I think we should add a getMime() method to nueserver later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maybe should to lower case here? otherwise, .PNG doesn't work

Agreed!

Nicely spotted TYPES in nueserver.js!

😉

getMime() method to nueserver

Sounds good

Not sure, but it would be probably good to expand the list with more common types.
It was quite interesting looking at a PDF in text format on the browser. Even though it is just the dev server.

Copy link
Collaborator Author

@nobkd nobkd Jan 28, 2024

Choose a reason for hiding this comment

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

getMime() method to nueserver

Maybe even a package, which has all the up-to-date types like mrmime or mime?

@tipiirai tipiirai merged commit b8124f0 into nuejs:master Jan 3, 2024
@tipiirai
Copy link
Contributor

tipiirai commented Jan 3, 2024

Thank you!

@nobkd nobkd deleted the fix/favicon-type branch January 3, 2024 07:35
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.

NueKit doesn't Generate Type for Favicon
3 participants