search

DDMAL / CantusDB

A new site for Cantus Database running under Django.
https://cantusdatabase.org
MIT License
5 stars 6 forks source link

Re-implement cantus index input tool #1538

Closed lucasmarchd01 closed 1 month ago

lucasmarchd01 commented 1 month ago

This PR re-implements the Cantus Index search view on the chant create page. This feature was turned off in https://github.com/DDMAL/CantusDB/pull/1142, and many users have been requesting the feature again.

The previous implementation parsed HTML from https://cantusindex.org/search API. Now, we use https://cantusindex.org/json-text/. This API is still not available on cantusindex.uwaterloo.ca, so we will have to switch over once it is.

We still have to write tests for this view, so I will convert that to a new issue to get this up and running sooner.

Chant Create: Screenshot 2024-06-17 at 12 26 54 PM CISearchView: Screenshot 2024-06-17 at 12 27 28 PM

Resolves #1391

dchiller commented 1 month ago

I find some things weird about the ci_search template, but am assuming you just re-added what we had before?

lucasmarchd01 commented 1 month ago

I find some things weird about the ci_search template, but am assuming you just re-added what we had before?

Yeah, I thought about making it look more like our other pages by adding base.html and so on but I think it was made this way to save space and scrolling time. The template is exactly the same as before but with some added javascript to handle populating the django-autocomplete-light selector for genre.

lucasmarchd01 commented 1 month ago

Also @dchiller (or @annamorphism ) check out https://github.com/DDMAL/CantusDB/issues/1391#issuecomment-2176157470 just to make sure this is how we'd like to implement this.

dchiller commented 1 month ago

Also @dchiller (or @annamorphism ) check out #1391 (comment) just to make sure this is how we'd like to implement this.

I think this is good for now ... If we figure out a way to increase the number of results in future then we make that a separate issue, but at least this feature will be available...

I do think the current message might therefore be a little misleading, so would agree about changing it to be something like the other option you suggested.