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

Support browserconfig.xml #66

Merged
merged 16 commits into from
Oct 12, 2017
Merged

Conversation

YoranBrondsema
Copy link
Contributor

@YoranBrondsema YoranBrondsema commented Sep 9, 2017

Generates browserconfig.xml according to the spec on https://msdn.microsoft.com/en-us/library/dn320426(v=vs.85).aspx. This targets Windows machines. It only populates the <tile> property, not <badge> or <notification>.

It will need some refactoring first if PR #65 gets merged first.

@YoranBrondsema YoranBrondsema changed the title [WIP] Support browserconfig.xml Support browserconfig.xml Sep 9, 2017
@san650
Copy link
Owner

san650 commented Sep 10, 2017

I've merged #65, can you update this PR to avoid using async/await in tests?

@YoranBrondsema
Copy link
Contributor Author

@san650 Done! We can keep the async/await as that test is executed in the browser and transpiled with Babel. However, I did need to polyfill Array.prototype.includes in the Node.js addition to the addon.

Copy link
Owner

@san650 san650 left a comment

Choose a reason for hiding this comment

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

To keep backward compatibility it would be great if we make this file to be opt-in instead of opt-out. So we don't override browserconfig.xml on projects that already have one.

The idea would be that by default we don't generate it and when the ms configuration is truthy we would generate it, with ms: true or ms: { ... }

What do you think @YoranBrondsema ?

README.md Outdated
@@ -383,6 +418,7 @@ icons: [
| `ms` | does not apply (for now)
| `android` | does not apply
| `favicon` | `<link rel="icon" href="/bar/fav.png" sizes="32x32">`
| `ms` | icon in `browserconfig.xml`
Copy link
Owner

Choose a reason for hiding this comment

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

Remove the reference to ms above (three lines above)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@YoranBrondsema
Copy link
Contributor Author

I agree! I'll make the changes.

@YoranBrondsema
Copy link
Contributor Author

@san650 I made the changes. If the ms property is falsey, the browserconfig.xml and associated meta-tag won't be generated.

I added the ms property to the blueprint; let me know if you prefer to not have it there as well.

@san650 san650 merged commit 1b0e28c into san650:master Oct 12, 2017
@san650
Copy link
Owner

san650 commented Oct 12, 2017

Thanks for contributing this feature @YoranBrondsema !!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants