# EmojiPicker stores state in global module-level variables
**URL:** https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056
**Category:** Bug
**Created:** [September 14, 2017, 3:44pm UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056 "2017-09-14T15:44:03Z")
**Posts on this page:** 14
**Page:** 1
### Author:  [@schungx](https://meta.discourse.org/u/schungx)
#### Post date: [September 14, 2017, 3:44pm UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/1 "2017-09-14T15:44:03Z")
In `emoji-picker.js.es6`:
The following variables are used for emoji-pickers, but they are currently module- **GLOBAL** variables instead of instance fields.
`let $picker, $filter, $results, $list, scrollPosition, $visibleSections, _checkTimeout;`
In particker, `$picker` is used to popup the emoji-picker. It is a module-global variable, so what that means is that there CANNOT BE any permanent emoji-picker’s on the page (e.g. `discourse/babble` and `discourse/retort` both put emoji-pickers permanently on the page).
When the compose-reply window is open, the standard emoji picker is created, overwriting `$picker` for everybody, causing all permanent emoji-pickers on the page to break.
Suggestion is to move all these variables into the class definition, and make them instance-specific fields. This should then allow for multiple emoji-pickers to co-exist on the same page with conflicts.
---
### Author:  [@codinghorror](https://meta.discourse.org/u/codinghorror)
#### Post date: [September 15, 2017, 12:24am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/2 "2017-09-15T00:24:30Z")
Question for @j.jaffeux
---
### Author:  [@j.jaffeux](https://meta.discourse.org/u/j.jaffeux)
#### Post date: [September 15, 2017, 4:32am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/4 "2017-09-15T04:32:18Z")
Yes it could probably use some refactoring, will look at it.
---
### Author:  [@schungx](https://meta.discourse.org/u/schungx)
#### Post date: [September 15, 2017, 4:43am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/5 "2017-09-15T04:43:53Z")
@j.jaffeux Thanks for looking into this.
When the code was written, it was probably assumed that only one emoji picker will ever be present on a page. A reasonable assumption actually, except that we’ll trust plugin authors to promptly break all our assumptions!
---
### Author:  [@j.jaffeux](https://meta.discourse.org/u/j.jaffeux)
#### Post date: [September 15, 2017, 4:47am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/6 "2017-09-15T04:47:32Z")
this and also I was quite new to js and discourse 😉
---
### Author:  [@schungx](https://meta.discourse.org/u/schungx)
#### Post date: [September 15, 2017, 5:08am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/7 "2017-09-15T05:08:41Z")
No worries mate. JS can be a b\*\*ch to pick up.
What you really need is to move those module-global variables into the class definition, so they are class-instance properties.
Then change all the access from `$picker` to `this.$picker` etc.
Then Bob’s your uncle! 🕶
P.S. I could have made the changes myself and submit a PR, but I got scared of forking the entire Discourse repo just to test such small changes…
---
### Author:  [@j.jaffeux](https://meta.discourse.org/u/j.jaffeux)
#### Post date: [September 15, 2017, 5:09am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/8 "2017-09-15T05:09:55Z")
No worries I will improve this. 😉
---
### Author:  [@schungx](https://meta.discourse.org/u/schungx)
#### Post date: [September 15, 2017, 5:21am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/9 "2017-09-15T05:21:05Z")
Your commit here is what’s breaking everything:
[https://github.com/discourse/discourse/commit/6a4d74d9f8681038ea1b273c173308b2fbd0f687#diff-2eadf5f693f654ff37f740c2904219a2](https://github.com/discourse/discourse/commit/6a4d74d9f8681038ea1b273c173308b2fbd0f687#diff-2eadf5f693f654ff37f740c2904219a2)
You attempted to optimize by caching some values, but caching them into module-global variables. Should have cached them into instance properties.
---
### Author:  [@schungx](https://meta.discourse.org/u/schungx)
#### Post date: [September 15, 2017, 6:21am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/10 "2017-09-15T06:21:28Z")
Oh, by the way, a not very related suggestion:
You are putting out the emoji’s as a huge bunch of PNG image files. When the emoji-picker pop’s up, the browser has to start fetching a huge number of very small PNG files. This affects user experience.
A much better idea is to have one single PNG image with all these sprites. Then use a class to fetch the relevant `background-position` from CSS. This should make the emoji’s show up much quicker.
So:
Standard emoji’s: put emoji name in class, use CSS to refer to background image and `background-position` to display it
Custom emoji’s: continue to put emoji name in `background-url`
---
### Author:  [@codinghorror](https://meta.discourse.org/u/codinghorror)
#### Post date: [September 15, 2017, 8:00am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/11 "2017-09-15T08:00:56Z")
> [@schungx](#):
>
> A much better idea is to have one single PNG image with all these sprites. Then use a class to fetch the relevant background-position from CSS. This should make the emoji’s show up much quicker.
Incorrect, sprite sheets are a HTTP/1.1 optimization. This is also a fragile and complex techinque that breaks, say, simply linking to a given emoji. HTTP 2, which we already use and have for quite a few months, doesn’t require these contortions.
[https://mattwilcox.net/web-development/http2-for-front-end-web-developers](https://mattwilcox.net/web-development/http2-for-front-end-web-developers)
Now the advice “only load what is (currently) visible” is timeless, and damn I wish web browsers had implemented this back in 2002. (Try loading a long web page with 50 images, when 49 of them are not visible due to scroll position.)
---
### Author:  [@schungx](https://meta.discourse.org/u/schungx)
#### Post date: [September 15, 2017, 8:29am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/12 "2017-09-15T08:29:49Z")
I agree, but old practices are there for a reason.
Not everybody runs HTTP/2. For example, those behind a corporate proxy may be stuck for 1.1 for a long time.
And it seems like those sites not running HTTPS are not served HTTP/2. There are still lots of sites not on HTTPS.
---
### Author:  [@codinghorror](https://meta.discourse.org/u/codinghorror)
#### Post date: [September 15, 2017, 9:24am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/13 "2017-09-15T09:24:50Z")
The default Discourse docker image runs http/2, see for yourself at `discourse.codinghorror.com` etc. [https://tools.keycdn.com/http2-test](https://tools.keycdn.com/http2-test)
This is one of the big, big advantages of Docker packaging – we can give people an optimal setup right out of the box.
---
### Author:  [@schungx](https://meta.discourse.org/u/schungx)
#### Post date: [September 15, 2017, 9:52am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/14 "2017-09-15T09:52:34Z")
@codinghorror You’re right! Using `Let's Encrypt` makes it really a no-brainer…
Since it is such a no-brainer to setup HTTP for Discourse, I agree that using HTTP/2 best practices is better, except in the case where somebody is behind a corporate proxy that is very very outdated and doesn’t support HTTP/2.
In other words, there really should not be many Discourse sites out there not running on HTTPS, right?
---
### Author:  [@j.jaffeux](https://meta.discourse.org/u/j.jaffeux)
#### Post date: [September 18, 2017, 10:59am UTC](https://meta.discourse.org/t/emojipicker-stores-state-in-global-module-level-variables/70056/15 "2017-09-18T10:59:31Z")
Fixed