-
Notifications
You must be signed in to change notification settings - Fork 5
Improve add to library dialog #791
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
base: master
Are you sure you want to change the base?
Conversation
|
@kelockhart Please see the video demo to see if that's a good way of selecting libraries. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #791 +/- ##
========================================
- Coverage 61.6% 61.6% -0.0%
========================================
Files 312 312
Lines 35995 35995
Branches 1593 1593
========================================
- Hits 22141 22139 -2
- Misses 13817 13819 +2
Partials 37 37 🚀 New features to boost your workflow:
|
|
Looking good! I would probably add checkboxes next to the libraries, though, as that's how we indicate selection everywhere else - see the author modal, for example. Otherwise the pagination and overall functionality looks right. |
|
Should we also provide a search bar so we can filter the library list? |
|
@thostetler We've taken out the ability to search the libraries, because it was a hacky method, not a built-in biblib method, and it made it so people with lots of libraries would have found this unusable. The pagination makes it usable for people with lots of libraries, plus people can sort by library name instead. |
|
@kelockhart I remember this now. Makes sense! Would be a good biblib feature then. |
|
@shinyichen looks good to me! |
thostetler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good, I have a small issue with the implementation -- but the idea is good and works well!
| for (let i = 0; i < ids.length; i++) { | ||
| const id = ids[i]; | ||
|
|
||
| editDocs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one issue I see here is that we're calling this one each of the IDs. I would expect the toast to keep triggering but it doesn't -- so that is good.
But I wonder what happens if one of the requests fails for some reason, would we get a toast about it? it looks like it would get swallowed by the other ones, if the final one succeeded.
It's a little nuanced, but I was able to replicate by blocking one of the library requests
library_issue-2026-02-09_14.18.41.mp4
I think instead of doing it this way with a loop we should use Promise.all and wait for all to be done and then report on number failed (and maybe state a reason) and number succeeded
wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya, let me fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thostetler Please check my latest commit
- Load/Show page 1 of libraries intially - Allow multiple library selections
82a32d3 to
5fc95fa
Compare

Screen.Recording.2026-02-04.at.4.30.28.PM.mov