internetarchive / openlibrary

One webpage for every book ever published!
https://openlibrary.org
GNU Affero General Public License v3.0
5.2k stars 1.36k forks source link

Lists created on search page initially appear checked in every dropper #9701

Open jimchamp opened 3 months ago

jimchamp commented 3 months ago

Background

Patrons are able to create lists of books using the "Create a new list" option found inside of the "My Books" droppers. Each of these droppers contain a number of actions that can be taken one a single book. Whenever a new list is created using the "Create a new list" option, the book that is associated with the dropper is automatically added to the new list and the list will appear in all "My Books" droppers on the page.

Problem

On pages where there are multiple "My Books" droppers, like our work search results page, creating a new list causes the new list to appear with a checkmark in every dropper. The checkmark indicates that the book associated with the dropper is on a list, and should only appear next to the list in the dropper where the list was created. For example, if I were to use the "My Books" dropper in the first search result to create a list named My Brand New List, an entry for My Brand New List should appear in all droppers, but only have a checkmark in the first dropper.

If the page is refreshed after creating a new list, the new list will only be checked in the expected dropper. This indicates that everything is working as expected on the server-side when a new list is created. The issue lies with the client-side code.

Evidence / Screenshot

Relevant URL(s)

https://openlibrary.org/search

Reproducing the bug

  1. Go to the /search page, and ensure that there are multiple search results.
  2. Using the "My Books" dropper found in the first search result, create a new list.

Context

Notes from this Issue's Lead

Proposal & constraints

The createDropdownListAffordance function is used to add an entry for a new list to a "My Books" dropper. This function has a boolean parameter named isActive, which, when true, indicates that the new entry should have a checkmark.

When a new list is created via a "My Books" dropper, the updateDroppersOnListCreation function is used to add the new list entry to every dropper on the page. isActive is initially set here for each dropper, and passed along to the dropper's createDropdownListAffordance function via onListCreationSuccess.

Using this information, determine why isActive is never false when these new list entries are created.

Related files

Code for the list entries in "My Books" droppers: https://github.com/internetarchive/openlibrary/blob/79dccb33ad74f3e9f16e69dce2bae7a568c8d3d0/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/ReadingLists.js

Code for list creation form: https://github.com/internetarchive/openlibrary/blob/79dccb33ad74f3e9f16e69dce2bae7a568c8d3d0/openlibrary/plugins/openlibrary/js/my-books/CreateListForm.js

Stakeholders


Instructions for Contributors

EdDiazGRS commented 3 months ago

Hi, I would love to take a crack at this. This would be a great first issue for me.

jimchamp commented 3 months ago

@EdDiazGRS, you have been assigned. Please let me know if you have any questions about anything.

Suswetha6 commented 2 months ago

Hey,can i work on this issue ?

jimchamp commented 2 months ago

@Suswetha6, this issue is already assigned to @EdDiazGRS.

@EdDiazGRS, please update me on your progress.

EdDiazGRS commented 2 months ago

@jimchamp , I'm currently working on it and I anticipated to be done by Friday.

TheBlueSummon commented 1 month ago

@jimchamp Could you please assign me to this issue, I would love to try fixing!!!

jimchamp commented 1 month ago

You've been assigned, @TheBlueSummon. Feel free to post any questions that you may have here.

TheBlueSummon commented 1 month ago

@jimchamp How do I test it locally to see if I actually fixed it (deploying the website locally ig)? I tried running $ docker compose run --rm home make test but after a while it just says "Validation passed!".

jimchamp commented 4 weeks ago
  1. Before making any code changes, try to reproduce the bug with your local instance.
  2. After reproducing the bug, make any necessary changes and rebuild the code.
  3. Reload the search page and attempt to reproduce the bug.
  4. Repeat steps 2 and 3 until you've arrived at a solution.
EmilyIsCoding commented 1 day ago

Hello, may I take on this issue? I believe it's a simple fix in openlibrary\plugins\openlibrary\js\my-books\CreateListForm.js line 127 where isActive and coverUrl need to be swapped to follow the order of onListCreationSuccess(listKey, listTitle, isActive, coverUrl).

jimchamp commented 9 hours ago

Go for it, @EmilyIsCoding.