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

Disable WP emojis polyfill #285

Closed
luboskmetko opened this issue Oct 10, 2017 · 10 comments
Closed

Disable WP emojis polyfill #285

luboskmetko opened this issue Oct 10, 2017 · 10 comments
Assignees

Comments

@luboskmetko
Copy link
Member

In default WP installation Emojis JS is loaded, we could disable it either by installing https://wordpress.org/plugins/disable-emojis/ plugin or by copying code from it to a new Chisel class. Thoughts?

@jakub300
Copy link
Collaborator

Why?

@luboskmetko
Copy link
Member Author

It ads additional DNS prefetch and another request but it's just a polyfill for older browsers. Emojis work even without it in modern browsers.

chisel-emojis

@luboskmetko luboskmetko changed the title Disable emojis Disable WP emojis polyfill Oct 10, 2017
@marcinkrzeminski
Copy link
Collaborator

👍 on that. Anyone recall if any of our customers did use that?

I would do it without the plugin if we decide on that. I could actually work on that if we decide to disable it.

@luboskmetko
Copy link
Member Author

Thanks, @marcinkrzeminski. I'm wondering if using the plugin wouldn't be better because if we want to do it properly we will have copy the code from the plugin anyway. Then we will have to maintain that code while with plugin it's up to its author https://wordpress.org/support/plugin/disable-emojis and you can easily update the plugin to fix issues.

Also deactivating plugin if someone needs that WP emojis polyfill is more flexible than updating the theme code.

@marcinkrzeminski
Copy link
Collaborator

Yeah there are always some up and downsides of each solution ;). I just like to keep the number of plugin as low as possible, my personal preference. The downside of going with plugin is that the customer could always disable the plugin ;). I guess we need to decide what's more important for us. Despite the solution we choose I'll be willing to implement it anyway. If we go for a plugin solution you think we should give the developer an option to install this plugin or not like we have for Gravity and other plugins.

@JakubSzajna
Copy link
Contributor

JakubSzajna commented Oct 11, 2017

I also prefer keeping number of plugins as low as possible. But if we will have to copy the code from the plugin to the chisel it does not make sense. What if plugin gets updated? You will update the code in old chisel project? WP will handle that automatically if it will be a plugin.
I'd say we install the plugin and activate it by default. Then developer can disable/remove it when he is sure he does not need it.

@marcinkrzeminski
Copy link
Collaborator

I'm OK with that ;). What about the others?

@luboskmetko
Copy link
Member Author

For me it's ok too :) We can later document which plugins are installed by default (Timber, obviously, and this one) and explain why.

@marcinkrzeminski
Copy link
Collaborator

👍 When we have a decision you can assign it to me and I'll work on it when heaving some time to spare ;)

@luboskmetko
Copy link
Member Author

@marcinkrzeminski thanks, I've invited you to collaborate on the project so I can assign this to you then :)

@marcinkrzeminski marcinkrzeminski self-assigned this Oct 11, 2017
@luboskmetko luboskmetko removed the wp label Oct 24, 2017
marcinkrzeminski pushed a commit to marcinkrzeminski/generator-chisel that referenced this issue Dec 4, 2017
Resolve xfiveco#285

Fix the issues reported by @jakub300
marcinkrzeminski pushed a commit to marcinkrzeminski/generator-chisel that referenced this issue Dec 4, 2017
luboskmetko pushed a commit that referenced this issue Dec 4, 2017
* Add Disable Emojis Plugin

Disable Emojis plugin is installed and activated with every WordPress project

* Fix issues after the review

Resolve #285

Fix the issues reported by @jakub300

* Indentation fix

Resolve #285
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants