www.fgks.org   »   [go: up one dir, main page]
Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Display lists patron's list in which the book is already added. #4597

Merged
merged 1 commit into from
Apr 27, 2021
Merged

Display lists patron's list in which the book is already added. #4597

merged 1 commit into from
Apr 27, 2021

Conversation

LeibTon
Copy link
Contributor
@LeibTon LeibTon commented Feb 13, 2021 •
edited

Closes #3630

Display the patron's list contatining a particular work underneath Want to Read Button.
This will help patron easily remove books from list already added.

Technical

Added id = "list-lists" alongwith class = "listsLists" in Line 296. This is because class = "listLists" is also used in Line 233. And we need to insert different lists in both ul. Hence separate query was needed.
Added a new parameter user_key to render_widget_add.
Apart from that, can modify the name of id and renderer in Line 233 from alreay-lists [It sounds ambiguous].

Testing

Open http://localhost:8080/books/OL7037695M/The_complete_works_of_Mark_Twain
The changes can be seen beneath Want to Read Button.

Screenshot

Screenshot from 2021-02-14 01-03-25

Stakeholders

@mekarpeles @jdlrobson

@LeibTon
Copy link
Contributor Author
LeibTon commented Feb 13, 2021

Since the patron's list is already displayed beneath Want to Read button. Should we remove it from Lists contaiing Lists?

@mekarpeles mekarpeles added the On Staging On staging.openlibrary.org label Feb 24, 2021
@mekarpeles
Copy link
Member
  1. When I add a book to my Reading Log (e.g. "Want to Read") The down arrow seems to have disappeared on staging: http://staging.openlibrary.org/books/OL7829676M/The_Giver
  2. when I test this feature, I don't see the widget -- only directly after I add a book on a list,. When I load the page or I refresh, it disappears

mekarpeles
mekarpeles requested changes Feb 24, 2021
View reviewed changes
Copy link
Member
@mekarpeles mekarpeles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see feedback in conversation

@mekarpeles
Copy link
Member

#4597 (comment)
"All Lists containing this book" is different from "my lists containing this book" and shouldn't be removed

@LeibTon
Copy link
Contributor Author
LeibTon commented Mar 4, 2021

While inspecting the element in http://staging.openlibrary.org/books/OL7829676M/The_Giver, I found out that the inspector was not able to load the image. So I clicked in the link for the image. Now, the link for the image opens in https://staging.... which doesn't exist. But when I change https to http in image link, it opens. So, can the dropdown icon be missing because, the base-url for image url has "https://staging.." whereas it should be "http://staging.."? The same can be the case for database, so the files are not actually updated into database, and hence after refreshing they disappears?

The url opened when I click for image url in inspector:
URL for image

@LeibTon LeibTon requested a review from mekarpeles March 10, 2021 07:54
@mekarpeles
Copy link
Member

re-adding to staging -- @prakashaditya369 do you recall where we left off on this one?

Is this issue still relevant?
#4597 (comment)

@mekarpeles
Copy link
Member

I think this one should be ok to go!
staging openlibrary org_works_OL15861476W_Glut

@mekarpeles mekarpeles merged commit a0637a5 into internetarchive:master Apr 27, 2021
@LeibTon
Copy link
Contributor Author
LeibTon commented Apr 28, 2021

I think this one should be ok to go!
staging openlibrary org_works_OL15861476W_Glut

The problem was with the dropdown-arrow. And I think it is still not appearing in the image given.

@cdrini cdrini mentioned this pull request Jun 2, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mekarpeles mekarpeles Awaiting requested review from mekarpeles

Assignees

@mekarpeles mekarpeles

Labels
On Staging On staging.openlibrary.org
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add current user's lists underneath Want to Read
2 participants
@LeibTon @mekarpeles