Bulk Search and List Creator #9279
Open
+729
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cdrini
requested changes
May 24, 2024
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
openlibrary/components/BulkSearch/components/BulkSearchControls.vue
Outdated
Show resolved
Hide resolved
openlibrary/components/BulkSearch/components/BulkSearchControls.vue
Outdated
Show resolved
Hide resolved
openlibrary/components/BulkSearch/components/BulkSearchControls.vue
Outdated
Show resolved
Hide resolved
openlibrary/components/BulkSearch/components/BulkSearchControls.vue
Outdated
Show resolved
Hide resolved
benbdeitch
force-pushed
the
7653/feature/BulkSearch
branch
from
b48ed3d
to
dee9e8c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #9279 +/- ##
==========================================
+ Coverage 15.89% 16.12% +0.23%
==========================================
Files 90 92 +2
Lines 4732 4768 +36
Branches 824 828 +4
==========================================
+ Hits 752 769 +17
- Misses 3468 3481 +13
- Partials 512 518 +6 ☔ View full report in Codecov by Sentry. |
for more information, see https://pre-commit.ci
benbdeitch
force-pushed
the
7653/feature/BulkSearch
branch
from
0540dbb
to
76ff1c3
Compare
benbdeitch
force-pushed
the
7653/feature/BulkSearch
branch
from
2867a54
to
d1115e3
Compare
for more information, see https://pre-commit.ci
cdrini
requested changes
Jun 12, 2024
| <template> | ||
| <div> | ||
| <BulkSearchControls :bulkSearchState="bulkSearchState" /> | ||
| <MatchTable :bulkSearchState="bulkSearchState" :listUrl = bulkSearchState.listUrl /> |
There was a problem hiding this comment.
Don't show the match table if there are no extracted books at the beginning
| <template> | ||
| <div class="book-card"> | ||
| <div class="cover"> | ||
| <img :src="coverImage"> |
There was a problem hiding this comment.
Suggested change
| <img :src="coverImage"> | |
| <img :src="coverImage" loading="lazy"> |
| <template> | ||
| <div> | ||
| <BulkSearchControls :bulkSearchState="bulkSearchState" /> | ||
| <MatchTable :bulkSearchState="bulkSearchState" :listUrl = bulkSearchState.listUrl /> |
There was a problem hiding this comment.
Add an <hr /> between these two
| textarea { | ||
| width: 100%; | ||
| display: flex; |
There was a problem hiding this comment.
Suggested change
| display: flex; | |
| display: flex; | |
| resize: vertical; | |
| box-sizing: border-box; |
|
|
||
|
|
||
| <template> | ||
| <details open> |
There was a problem hiding this comment.
Give this a class name like bulk-search-controls and give it padding: 20px in style. That should tighten up this UI enough I reckon!
| <option value="4">GPT</option> | ||
| </select></label> | ||
| <label v-if="bulkSearchState.extractionOptions.use_gpt">API-Key: | ||
| <input type="text" v-model="bulkSearchState.extractionOptions.api_key" /> |
There was a problem hiding this comment.
Suggested change
| <input type="text" v-model="bulkSearchState.extractionOptions.api_key" /> | |
| <input type="password" v-model="bulkSearchState.extractionOptions.api_key" /> |
| }, | ||
| data() { | ||
| return { | ||
| OL_SEARCH_BASE: 'openlibrary.org', |
| }, | ||
| data() { | ||
| return { | ||
| OL_SEARCH_BASE: 'openlibrary.org', |
| <td>{{ bookMatch.extractedBook.author }}</td> | ||
| <td> | ||
| <div class="bookCards"> | ||
| <a :href="searchUrl">L</a> |
There was a problem hiding this comment.
Suggested change
| <a :href="searchUrl">L</a> | |
| <a :href="searchUrl" title="View results in Open Library">🔎</a> |
| </label> | ||
| <SampleBar @sample="(msg) => bulkSearchState.inputText = msg" /> | ||
| <label> | ||
| <input checked v-model="bulkSearchState.matchOptions.includeAuthor" type="checkbox" /> Use author in |
There was a problem hiding this comment.
Shouldn't specify this since it's defined by includeAuthor
Suggested change
| <input checked v-model="bulkSearchState.matchOptions.includeAuthor" type="checkbox" /> Use author in | |
| <input v-model="bulkSearchState.matchOptions.includeAuthor" type="checkbox" /> Use author in |
cdrini
added
the
Needs: Submitter Input
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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