Client side keyword detection #48
Conversation
- Detects certain keywords on the client side and uses them as input to the ad decision. Uses the top 10 most common keywords. - Uses detected keywords in addition to any manually specified ones - Tries to find the main content and not use navigation in the keyword detection. - Keyword detection is pretty fast and take no more than single digit milliseconds
This is required for IE11 to work correctly. This adds ~7kb to the minified bundle and ~1kb after gzip.
| if (a[1] < b[1]) return 1; | ||
| return 0; | ||
| } | ||
| ).slice(0, MAX_KEYWORDS).map((x) => x[0]); |
There was a problem hiding this comment.
This is so functional it makes my brain hurt. The final map is basically just pulling the value from a 'keyword': count dict item?
There was a problem hiding this comment.
Yes. keywordHist is a dict with keys as the keyword and the value is the count of its occurrences. Object.entries is basically the same as dict.items in Python. It returns an array of length 2 arrays (key, value). We then sort that array such that the most common items are first. Then we take the first MAX_KEYWORDS items from the array so as to only get the top X keywords. Lastly, we drop the number of occurrences and just return the keywords.
| // Additional categorization can be done on the server side for pages | ||
| // that request ads commonly but this quick and easy categorization | ||
| // works decently well most of the time. | ||
| const KEYWORDS = { |
There was a problem hiding this comment.
Could use a mapping -- I assume the key is the page copy and the value is the server-side keyword?
| @@ -58,8 +182,11 @@ export class Placement { | |||
|
|
|||
| this.response = null; | |||
|
|
|||
| this.keywords = keywords || ""; | |||
| this.campaign_types = campaign_types || "paid|community|house"; | |||
| this.keywords = (keywords || []).concat(this.detectKeywords()); | |||
There was a problem hiding this comment.
It might be useful on the server-side to have the user-defined & automatic keywords separate in the request. Maybe we don't need it for a v1 given that it requires server changes (eg. automatic_keywords alongside keywords), but I do think we might want to treat them differently in the future.
There was a problem hiding this comment.
Ya, I could see that. It's possible also that this client side stuff is only temporary too. We'll have to see.
Polyfill URL and URLSearchParams
| @@ -92,7 +219,7 @@ export class Placement { | |||
| } | |||
|
|
|||
| let classes = (element.className || "").split(" "); | |||
| if (classes.includes("loaded")) { | |||
| if (classes.indexOf("loaded") >= 0) { | |||
There was a problem hiding this comment.
Just a note that includes requires a polyfill in IE11. That's why I removed it.
<main>thenrole="main"then just<body>