bayang / jelu

Self hosted read and to-read list book tracker
MIT License
369 stars 14 forks source link

Detect if ISBN field contains ASIN for Metadata search #113

Closed DarthNerdus closed 7 months ago

DarthNerdus commented 7 months ago

ASIN search PR

This PR allows a user to paste in the ASIN of a book into the ISBN field.

I initially made changes to add an ASIN field, but that felt like a much larger change for the same effect and added unnecessary UI bloat. An argument could be made for updating the label to indicate that ASIN is supported, but for now I implemented this as an "easter egg". If you type in an ASIN, instead of failing because fetch-ebook-metadata was explicitly told it was an ISBN, it will now check if it is an ASIN and proceed with the same metadata lookup.

Other items in my fork

I do want to briefly tell you what my plan is for many of my little edits. For small, atomic changes I'll be opening PRs. But I have other things I've changed to my preference (default pagination size) or added functionality that I'm unsure whether it fits the goal of the project. For those, I am maintaining a fork/branch that I am constantly rebasing against your main branch whenever you push changes. In this way, I'm maintaining a set of atomic patches you are free to pull in (or I will PR) and others can use. A few examples right now, if you'd like me to push them up too:

bayang commented 7 months ago

Ok, that looks nice, thanks for your work on Jelu. For backend related code I would like some tests please, frontend obviously should be tested as well but I don't have enough time, so I try to at least maintain a well tested backend codebase.

For your other features :

DarthNerdus commented 7 months ago

Updated to add tests asserting the three different test paths match correctly.

DarthNerdus commented 7 months ago

@bayang -- Here's a loom video of the feature to jump from search to add.

I can submit a PR without the focusing that you see happening. I like the focusing because it allows me to progress between the views via keyboard, not mouse. I doubt that'd be preferable for everyone though.

No sweat if this isn't wanted, was definitely more of a "me" thing I can maintain.

EDIT: ...it helps to link the video... 🤦🏻

https://www.loom.com/share/3e8f79981445434e83f9e9cdab4e9b00?sid=a5fc033e-c7e3-433b-ac2f-162e28f0ead0

bayang commented 7 months ago

:tada: This PR is included in version 0.55.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

bayang commented 7 months ago

@DarthNerdus the feat in your video looks nice, you can submit a PR as well if you want.