{"action":"created","issue":{"url":"https://api.github.com/repos/Juris-M/citeproc-js-docs/issues/1","repository_url":"https://api.github.com/repos/Juris-M/citeproc-js-docs","labels_url":"https://api.github.com/repos/Juris-M/citeproc-js-docs/issues/1/labels{/name}","comments_url":"https://api.github.com/repos/Juris-M/citeproc-js-docs/issues/1/comments","events_url":"https://api.github.com/repos/Juris-M/citeproc-js-docs/issues/1/events","html_url":"https://github.com/Juris-M/citeproc-js-docs/pull/1","id":153680327,"number":1,"title":"[WIP] - Example code collaboration","user":{"login":"dsifford","id":5240018,"avatar_url":"https://avatars.githubusercontent.com/u/5240018?v=3","gravatar_id":"","url":"https://api.github.com/users/dsifford","html_url":"https://github.com/dsifford","followers_url":"https://api.github.com/users/dsifford/followers","following_url":"https://api.github.com/users/dsifford/following{/other_user}","gists_url":"https://api.github.com/users/dsifford/gists{/gist_id}","starred_url":"https://api.github.com/users/dsifford/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dsifford/subscriptions","organizations_url":"https://api.github.com/users/dsifford/orgs","repos_url":"https://api.github.com/users/dsifford/repos","events_url":"https://api.github.com/users/dsifford/events{/privacy}","received_events_url":"https://api.github.com/users/dsifford/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":11,"created_at":"2016-05-08T22:10:21Z","updated_at":"2016-05-09T18:02:07Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/Juris-M/citeproc-js-docs/pulls/1","html_url":"https://github.com/Juris-M/citeproc-js-docs/pull/1","diff_url":"https://github.com/Juris-M/citeproc-js-docs/pull/1.diff","patch_url":"https://github.com/Juris-M/citeproc-js-docs/pull/1.patch"},"body":"### Running List of Changes\r\n- Add `.gitignore` file for `_build` directory\r\n- Add ESLint following airbnb style guide (with slight modifications)\r\n- Create separate file for ES6 changes.\r\n\r\n\r\n### Various notes as I walk through the code\r\n- [This line](https://github.com/dsifford/citeproc-js-docs/blob/master/_static/js/citesupport.js#L166) in the code is technically incorrect, but Firefox and Chrome will fix it for you. This should always evaluate to false because `classList` does not exist on `HTMLNode`. ClassList only exists on `HTMLElement`. Writing it instead like this is the correct form: \r\n```js\r\nconst sib = menuElement.parentElement.nextElementSibling;\r\n```\r\n- Variable declared without any `var`, `let`, or `const` keyword [here](https://github.com/dsifford/citeproc-js-docs/blob/master/_static/js/citesupport.js#L94)\r\n- Functions should each satisfy **one** job. Composing functions within functions is dangerous, hard to debug, and makes following the code super confusing. (Example [here](https://github.com/dsifford/citeproc-js-docs/blob/master/_static/js/citesupport.js#L102))\r\n- Having functions who's names are nearly identical (differing only in plurality) is confusing. (Example: `domRemoveCitation` vs `domRemoveCitations`)\r\n- There are several functions that serve to complete a small part of a larger purpose. It would be more readable (and easier to follow) if these were combined into one function (e.g. a single function to update the DOM after a change is made)\r\n- TBH, I think this example fails the people who are trying to learn from it because it's too easy to get lost in all the DOM-related functions. There's just too much random, unnecessary logic going on to be able to understand the real issue, which is how does one take citeproc-js and use it for inserting inline-citations. Using react for this to manage the state would probably shrink the DOM functions by a minimum of 50%, but there will still be some boilerplate that people will have to understand before getting to the main issue. Perhaps making a minimal example as well would be best here.. Something that doesn't generate and destroy popup menus and isn't fancy. I'm thinking perhaps having a single line of text and above it showing the citation selection menu. Trigger the business logic with a simple `onChange` handler. Done.\r\n\r\n#### Moral of the story, and conclusion I came to tonight.\r\n- I think I might abandon trying to rewrite your code and just write a minimal example that you can choose to either include or not in your documentation.\r\n\r\n\r\n### Confusion Remains...\r\n\r\n- I'm still not entirely sure what the purpose of all this logic here is. Can you explain to me what the goal is? It feels like it's overly complex for the sake of being overly complex.\r\n\r\n```js\r\nfunction getCurrentCitationInfo() {\r\n        debug('getCurrentCitationInfo()');\r\n        var info = {\r\n            pos: 0,\r\n            citationID: null\r\n        };\r\n        var citemeNodes = document.getElementsByClassName('citeme');\r\n        var citationIndex = 0;\r\n        for (var i=0,ilen=citemeNodes.length;i<ilen;i++) {\r\n            var node = citemeNodes[i];\r\n            var sib = node.nextSibling;\r\n            var hasCitationNode = false;\r\n            if (sib && sib.classList && sib.classList.contains('citation')) {\r\n                hasCitationNode = true;\r\n            }\r\n            if (node.firstChild) {\r\n                info.pos = i;\r\n                info.citationIndex = citationIndex;\r\n                if (hasCitationNode) {\r\n                    info.citationID = sib.getAttribute('id');\r\n                }\r\n                break;\r\n            } else if (hasCitationNode) {\r\n                citationIndex++;\r\n            }\r\n        }\r\n        return info;\r\n    }\r\n```\r\n\r\nis `citationIndex` any different than simply `i` in this case? \r\n\r\nwont they all `haveCitationNode` since you're iterating an HTML collection OF citation nodes? What is the purpose of citationIndex?"},"comment":{"url":"https://api.github.com/repos/Juris-M/citeproc-js-docs/issues/comments/217940672","html_url":"https://github.com/Juris-M/citeproc-js-docs/pull/1#issuecomment-217940672","issue_url":"https://api.github.com/repos/Juris-M/citeproc-js-docs/issues/1","id":217940672,"user":{"login":"dsifford","id":5240018,"avatar_url":"https://avatars.githubusercontent.com/u/5240018?v=3","gravatar_id":"","url":"https://api.github.com/users/dsifford","html_url":"https://github.com/dsifford","followers_url":"https://api.github.com/users/dsifford/followers","following_url":"https://api.github.com/users/dsifford/following{/other_user}","gists_url":"https://api.github.com/users/dsifford/gists{/gist_id}","starred_url":"https://api.github.com/users/dsifford/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dsifford/subscriptions","organizations_url":"https://api.github.com/users/dsifford/orgs","repos_url":"https://api.github.com/users/dsifford/repos","events_url":"https://api.github.com/users/dsifford/events{/privacy}","received_events_url":"https://api.github.com/users/dsifford/received_events","type":"User","site_admin":false},"created_at":"2016-05-09T18:02:07Z","updated_at":"2016-05-09T18:02:07Z","body":"> If you decide at some point that you would like the citationIdToPos mapping\r\nstuff stripped out,  say the word: I know what needs doing for that, it\r\nwould just take a little time to set up.\r\n\r\nTrying to figure out how to make a new tab to the sidebar now. \r\n\r\nI'd like to see if I can make this into a workable simple React app. That would be easiest (for me) to remove a majority of the DOM cruft and also to make it consumable by others.\r\n\r\nTried adding a new `.rst` file and adding it to the list in `index.rst` but it didn't seem to build"}}