Skip to content
This repository has been archived by the owner on Jan 2, 2018. It is now read-only.

Default favicon #58

Closed
sgulseth opened this issue Dec 24, 2016 · 4 comments
Closed

Default favicon #58

sgulseth opened this issue Dec 24, 2016 · 4 comments

Comments

@sgulseth
Copy link

The roc.config provided with this extension(https://github.com/rocjs/roc-package-web-app-react/blob/master/extensions/roc-package-web-app-react/src/config/roc.config.js#L14-L17) has a default favicon.png. If only setting helmet-head resources through the React-component you end up with two favicons:

screenshot 2016-12-24 15 39 54

Helmet-component:

<Helmet
                    link={[
                        { rel: 'stylesheet', href: 'https://fonts.googleapis.com/css?family=Montserrat' },
                        { rel: 'stylesheet', href: '/css/loaders.css' },
                        { rel: 'icon', href: '/img/favicon.png' }
                    ]}
                />

Temp fix is to manually override settings.runtime.head.link

@dlmr
Copy link
Member

dlmr commented Dec 24, 2016

Thanks for the report! We had good intentions with this default in the configuration, pushing for convention over configuration, but seems it might be best to remove it to avoid this problem since we have seen this before.

We could also check so only a single icon is used but that is probably complexity that gives little value.

@sgulseth
Copy link
Author

I see. Wouldn't it be "enough convention" by providing the favicon setup through the bootstraping?

@dlmr
Copy link
Member

dlmr commented Dec 24, 2016

I think that would probably be best yes. A PR that changes this would be appreciated. 🙂

We will also need to update our templates to match this change.

@dlmr
Copy link
Member

dlmr commented Jan 2, 2018

This issue has been moved into the mono repository rocjs/extensions#22.

@dlmr dlmr closed this as completed Jan 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants