-
Notifications
You must be signed in to change notification settings - Fork 88
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
Serve widgets minified for production + new API for widgets and optional search #490
Serve widgets minified for production + new API for widgets and optional search #490
Conversation
davidbgk
commented
Jun 17, 2016
•
edited
Loading
edited
- From 19.2Kb to 9.43Kb, not bad :) Actually more 14.4Kb with added features, before gzip
- New API to load on demand the datasets, useful for territories' external pages
- New optional search box to filter retrieved datasets on the client side
{'base': 'e', 'letters': /[\u0065\u24D4\uFF45\u00E8\u00E9\u00EA\u1EC1\u1EBF\u1EC5\u1EC3\u1EBD\u0113\u1E15\u1E17\u0115\u0117\u00EB\u1EBB\u011B\u0205\u0207\u1EB9\u1EC7\u0229\u1E1D\u0119\u1E19\u1E1B\u0247\u025B\u01DD]/g}, | ||
{'base': 'i', 'letters': /[\u0069\u24D8\uFF49\u00EC\u00ED\u00EE\u0129\u012B\u012D\u00EF\u1E2F\u1EC9\u01D0\u0209\u020B\u1ECB\u012F\u1E2D\u0268\u0131]/g}, | ||
{'base': 'o', 'letters': /[\u006F\u24DE\uFF4F\u00F2\u00F3\u00F4\u1ED3\u1ED1\u1ED7\u1ED5\u00F5\u1E4D\u022D\u1E4F\u014D\u1E51\u1E53\u014F\u022F\u0231\u00F6\u022B\u1ECF\u0151\u01D2\u020D\u020F\u01A1\u1EDD\u1EDB\u1EE1\u1EDF\u1EE3\u1ECD\u1ED9\u01EB\u01ED\u00F8\u01FF\u0254\uA74B\uA74D\u0275]/g}, | ||
{'base': 'oe', 'letters': /[\u0153]/g}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
territoryElement.innerHTML = initialDatasets | ||
function filterDatasets (territoryElement, event, datasets, initialDatasets) { | ||
const searchValue = event.target.value | ||
if (searchValue.length >= 3) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Early return here can save an indentation for the reminder of the function.
@@ -272,6 +278,7 @@ function insertSearchInput (event, territoryElement) { | |||
searchNode.style.margin = '1em' | |||
searchNode.style.padding = '.1em' | |||
territoryElement.parentNode.insertBefore(searchNode, territoryElement) | |||
// TODO: add a timeout between keyups. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://css-tricks.com/debouncing-throttling-explained-examples/ :) Use lodash :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding lodash.debounce
adds 5Kb to my build vs. 10 lines of unbloated ES6 that I understand. Not worth it IMHO.
r+wc |
* it stops being called for `wait` milliseconds. Inspired by: | ||
* https://davidwalsh.name/javascript-debounce-function | ||
*/ | ||
function debounce (func, wait = 350) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a debounce
dependency (I can't say if it's still in use or not). It might be interesting to either drop the useless dependency and use this piece everywhere (or reuse the dependency)