Bulk Search and List Creator #9279
Conversation
This reverts commit 3355bb4.
There was a problem hiding this comment.
Nice great work @benbdeitch ! Did a first pass 👍
Main notes:
- I think you forgot to add the
BulkSearch.vuefile! 😁 - Can you format the vue files? I'm not sure why it's not causing eslint troubles 🤔 But it's a little hard to read. I'd recommend installing the Vue offical VS code extension ( https://marketplace.visualstudio.com/items?itemName=Vue.volar ) and then for each file, run auto form
shift-alt-f
| return `https://covers.openlibrary.org/b/id/${this.doc.cover_i}-M.jpg` | ||
| }, | ||
| workUrl() { | ||
| return `https://openlibrary.org/books/${this.doc.key.split('/')[2]}` |
There was a problem hiding this comment.
Should this be /works ?
| return `https://openlibrary.org/books/${this.doc.key.split('/')[2]}` | |
| return `/works/${this.doc.key.split('/')[2]}` |
There was a problem hiding this comment.
Books and works are interchangeable, so far as I recall. I think you're right though, this could probably be https://openlibrary.org/${this.doc.key}, from what I'm looking at.
|
|
||
|
|
||
|
|
||
| <template> |
There was a problem hiding this comment.
The template file should be at the top. We tend to do template, script, then css
| <details open> | ||
|
|
||
| <summary>Input</summary> | ||
| <div> |
There was a problem hiding this comment.
This indenting needs to be cleaned up; auto-format all these files. I'm surprised we don't have a linter for these already configured!
|
|
||
| <script> | ||
|
|
||
| import sampleBar from './SampleBar.vue' |
There was a problem hiding this comment.
We make components in UpperCamelCase.
| import sampleBar from './SampleBar.vue' | |
| import SampleBar from './SampleBar.vue' |
| <button @click="extractBooks">Extract Books</button> | ||
| <button @click="matchBooks">Match Books</button> | ||
| </div> | ||
| <div class="ErrorBox" v-if="this.bulkSearchState.errorMessage"> |
There was a problem hiding this comment.
Our class names are generally kebab-case, but it seems like this class is unused, so can drop it.
| <div class="ErrorBox" v-if="this.bulkSearchState.errorMessage"> | |
| <div v-if="this.bulkSearchState.errorMessage"> |
There was a problem hiding this comment.
Let's name this MatchRow ; Table is a little vague.
|
|
||
| td,th { border: 1px solid; padding: 4px; } | ||
| .bookCards { | ||
| font-family: Roboto, sans-serif; |
There was a problem hiding this comment.
We shouldn't need to set the font-family here, it should be inherited.
| font-family: Roboto, sans-serif; |
| constructor(){ | ||
| /** @type {string} */ | ||
| this.inputText= ''; | ||
| /** @type {ExtractedBooks[]} */ |
There was a problem hiding this comment.
| /** @type {ExtractedBooks[]} */ | |
| /** @type {ExtractedBook[]} */ |
There was a problem hiding this comment.
This can just be in bulk_search.html ; it's not a demo for long! ;)
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Closes #7653
With this PR, the long-requested Bulk Search feature is finally functional. It enables users to extract books and titles from sections of text, and search the Open LIbrary database for each pair, at the click of a button. ChatGPT-based extraction is functional, but requires the user to supply their own key for that purpose.
Technical
The majority of the changes involve the addition of a new Vue component. Rather than conveying information between individual components by event watchers, information is instead conveyed through a state defined in
/openlibrary/components/BulkSearch/utils/classes.js.Currently, the 'create a new list' function is not supported, but I intend to resolve that within the next few days.
Unfortunately, there were some limitations with how the table displays. Horizontal scrolling will likely be needed to navigate it, in cases where there are a large amount of works retrieved by a given search. This will likely be addressed in a future pull request.
Testing
Simply use the component, which is located at
localhost:8080/bulk/search.Screenshot
Stakeholders
@cdrini