-
Notifications
You must be signed in to change notification settings - Fork 34
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
Keywords #35
base: master
Are you sure you want to change the base?
Keywords #35
Conversation
Added buttons for previous and next search results.
…on in the tree-data.
Removed commented code.
…n findNodes method. Updated index.html to use bootstrap-treeview.js.
… to bootstrap-treeview.min.js.
@plandes Do you want me to address anything in this request? Maybe I missed the mark on what you wanted me to do? |
@jarrielcook Sorry I haven't been able to get to that pull request. I've been busier than usual and your pull request is large and not trivial to understand and be able to fully test. I haven't forgotten about it and will eventually get to it. |
First, sorry it's taken me so long to get back. Things have slowed down and I finally have time to address your generous pull request. I like the arrow buttons and of course searching on keywords is a great addition! However, I have some other concerns (below). Can you address them and then submit a new pull request? SecurityThere's still a minified version of a javascript file in commit 6d463e4. Sorry if I wasn't clear, but I just can't have any version of any minified file in any commit of the source for security reasons. Can you please just use the non-minified version and refer to that (as you did in b1b818f)? Later I can minify it myself and then refer to the min version. Sorry to have to ask this--if the pull request were smaller I'd cherry pick commits, but that's problematic for a lot of reasons (esp given other concerns below). It would probably be easiest to just remove that commit from your fork (remove it locally and the do a mirror push). Then resubmit the pull. Keywords Metadata BloatIn just one paper, the keywords output, which are added from the automatic tags, exceeds 11K characters. For users that have very large libraries of papers, will the browser be able to process the data? Have you done testing on large libraries? Already in my library, the search takes a very long time, and this will certainly add to that. Can we make this optional with the default to be left off? Then a user can add in the config file to add the keywords. It would be also to be able to enable it as a command line argument so that users can enable keyword searches for partial exports. I can add the command line feature if you don't have time. BugIt seems like there's a bug: when you search for a keyword on an item, even the Clear button doesn't remove the selection. Also, the PDF stops rendering in the right pane. The rendering in the right pane might be something else, but in the your version I confirmed that even the "view" button creates a new tab/window with just "blank". The issue of rendering with the "View" seems to happen even without clearing the selection. QuestionsJust curious: what is the |
Added a keyword search feature that extracts keywords from the Zotero database during "zotsite export" and adds the keywords to the "metadata" field in "zotero-site/js/zotero-tree-data.js"
This feature required a change to bootstrap-treeview. I grabbed bootstrap-treeview.js from https://github.com/jonmiles/bootstrap-treeview. I first added the non-minimized and unchanged version to 'resources/site/lib/js' and updated 'resources/site/src/index.html' to use it. Then I modified findNodes in bootstrap-treeview.js to search the 'metadata' attribute rather than the 'text' attribute. Next, I minimized bootstrap-treeview.js to bootstrap-treeview.min.js and reverted 'resources/site/src/index.html' to use the minimized version again.