jonathantneal / svg4everybody

Use external SVG spritemaps today
https://jonneal.dev/svg4everybody/
Other
3.29k stars 353 forks source link

Proposed criteria for accepting new features #137

Open shawnbot opened 7 years ago

shawnbot commented 7 years ago

An interesting feature was proposed in #90, and I suggested that we decline to add it because:

I'm personally not a fan of workarounds for either security controls or standard behaviors. svg4everybody is a polyfill, which means that it is intended to upgrade implementations to support the standard behavior. Because this feature introduces behavior that deviates from the standard, I don't think that it belongs here.

For this issue and more generally, I'd like to suggest criteria for deciding whether to accept features like this in the future:

Does the input SVG work as intended in standard implementations without the feature?

If the answer is no, then I think the feature should be rejected. One counter-argument is that users may pass an option to force the shim, but I would argue that this is bad practice because it penalizes users of modern browsers and effectively prevents JS-free users from seeing the SVGs.

Whatever we decide should go into CONTRIBUTING.md and possibly into an issue or pull request template so that contributors know what to expect when they propose a new feature.

timeiscoffee commented 7 years ago

Absolutely agreed! As the first line of README states,

SVG for Everybody adds SVG External Content support to all browsers. It shouldn't do any more than that. (of course, there could be exceptions...)