Thanks! In general this looks great, but some higher level concerns:

• We still have some users on IE and older Safari versions and I think we'd prefer to not break things for them by using ES6 javascript like .includes and arrow assignments (those were the ones I spoted; there may be others).

• Instead of your own custom function for getting text using selectors, please use the attr and text functions as documented here: https://www.mediawiki.org/wiki/Citoid/Creating_Zotero_translators#Common_code_blocks_in_translators -- doing this uniformly will make maintenance easier and they'll eventually ship with Zotero.

• Please add an AGPL license: https://www.mediawiki.org/wiki/Citoid/Creating_Zotero_translators#License_block

• The expectation is for translators to also work on multiple items, e.g. search results. I haven't looked at the site, but unless this is prohibitively complicated, please add that functionality.

Thanks again!

We still have some users on IE and older Safari versions and I think we'd prefer to not break things for them by using ES6 javascript like .includes and arrow assignments (those were the ones I spoted; there may be others).

We're now pretty comfortable allowing ES6 for new development. We don't need to go out of our way to break existing translators, particularly core ones, but Safari 10, which supports most ES6 features (certainly let, arrow functions, and String.includes()), can be installed on Macs going back to 2007-2009. (The Safari connector itself now requires macOS 10.11.) iOS 10, which included Safari 10, installs back to the iPhone 5 from five years ago. And I don't think we need to make any development decisions based on the bookmarklet in IE.

Additionally, please also add some test cases.

OK, thanks Dan. @petervelosy ignore the part about ES6 then, but the other issues do still apply, as does the request for test cases per @zuphilip

Please check if this all makes sense. We'd want the extra information in a note, not the Extra field, so I moved that. Apart from that I just fixed things and added multiples & tests (please do test your code before comitting, though; what you had didn't run at all)

Thank you for all these changes and the clarification, it all makes sense now. Is there anything else I need to do, or is this translator ready for merging?

P.s. The last comment was by me, but a colleague was logged in with his GitHub account on this PC.

This PR is now merged and the new translator will get deployed now 🚀 .

Thank you very much @petervelosy!