Skip to content

Show Want to Read button on trending pages#12833

Open
Anuragp22 wants to merge 1 commit into
internetarchive:masterfrom
Anuragp22:12810/fix/want-to-read-button-trending
Open

Show Want to Read button on trending pages#12833
Anuragp22 wants to merge 1 commit into
internetarchive:masterfrom
Anuragp22:12810/fix/want-to-read-button-trending

Conversation

@Anuragp22
Copy link
Copy Markdown

Closes #12810

Add the Want to Read button to trending pages. [fix]

Technical

Trending pages render each book through the SearchResultsWork macro but did not pass include_dropper=True, so the reading-log dropper (the Want to Read button) never rendered. Every other listing page (work_search.html, type/author/view.html, type/list/view_body.html, macros/FulltextResults.html) passes it. This adds include_dropper=True to the macro call in trending.html. Trending feeds Solr work documents via Bookshelves.add_solr_works (which include edition_key and logged_edition) and the dropper loads asynchronously, so no server-side changes are required.

Testing

Visit /trending/now while logged in: a Want to Read dropper appears on each book, matching search results and list pages. When logged out it renders in its disabled state, consistent with other listing pages.

Screenshot

Pending: current master does not start locally (see #12827), so a running screenshot could not be captured. The change mirrors the established use of include_dropper on other listing templates.

Stakeholders

@mekarpeles

Pass include_dropper=True to the SearchResultsWork macro in trending.html
so the reading-log dropper renders, matching work_search, author, and
list pages. Trending feeds Solr work docs (with edition_key) via
add_solr_works and the dropper loads async, so no server-side changes are
needed.

Closes internetarchive#12810
@mekarpeles
Copy link
Copy Markdown
Member

Thanks for the contribution, @Anuragp22!

🤖 Copilot has been assigned for an initial review.

@mekarpeles is assigned to this PR and currently has:

  • 0 open PR(s) of equal or higher priority to review first
PR triage checklist (maintainers / Pam)
  • PR description — not empty; explains what the change does and how to verify it
  • References an issue — PR body contains a #NNN reference
    • Linked issue is triaged — has a Priority: * label (not just Needs: Triage)
    • Linked issue is assigned — has at least one assignee
  • Commit history clean — no WIP/fixup/conflict noise; commit messages are meaningful
  • CI passing — no failing check-runs
  • Test cases present — if the change touches substantive logic, test coverage exists or is explained
  • Proof of testing — PR body includes a description of what was tested, a screenshot, or a video

Note

This comment was automatically generated by Pam, Open Library's Project AI Manager, on behalf of @mekarpeles. Pam is designed to provide status visibility, perform basic project management functions and relevant codebase research, and provide actionable feedback so contributors aren't left waiting.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes missing reading-log UI on Trending pages by enabling the existing “Want to Read” dropper in the shared SearchResultsWork rendering path. This aligns /trending/* with other listing pages which already pass include_dropper=True.

Changes:

  • Pass include_dropper=True to the macros.SearchResultsWork(...) call in trending.html so the reading-log dropper renders for each work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Want to read button missing from trending pages

3 participants