List Card Async Follow Functionality #11311

mekarpeles · 2025-09-25T06:51:40Z

Refactor

Technical

I made several changes to improve the code that I wrote for #11093 which was a refactor of #9509. Our goal is to make the action of following new users as seamless as possible and the fixes in this PR help refactor some of the code written to implement the async following functionality so that users don't get redirected to profile page of the user that they just followed or unfollowed.

The main changes that were made in this PR:

Added internationalization
Disabled the follow buttons after one was clicked and enabled after the follow/unfollow request was completed
Renamed several variables for consistency/readability
Created a separate initAsyncFollowing method to make the initListSection more concise and the book-page-lists.js file easier to read
Changed the ajax request to a fetch request.
Removed a lot of the declared variables prior to making the POST request and instead used the form as the input for the POST request
Instead of indexing a class from a button which is easily breakable, I queried the class using contains instead.
Reverted the code in api.py so that we can address the issue in it in another PR

Change I did not make:

Refactor list follow card #11093 (comment)
This is needed in case there are multiple lists by the same publisher and so that the follow/unfollow buttons change in other lists as well. I am open to suggestions on improving this code in case this can be rewritten.

Testing

Copied from #10926:

Create a test account
Create a test list and add a book to it under the test account
Return to main account
Go to book page of the book that has been added to the test list
Follow user by clicking on the follow button on the list card.
Follow icon will negate and the page won't refresh/redirect
Go to the user's account in a separate tab and confirm that follow/unfollow action has been registered.

Screenshot

https://www.loom.com/share/ecca57f343a34bd1a6185f8cd6266acb?sid=0d6b52ad-93fa-4483-bd30-58e8a1b07324

Stakeholders

@mekarpeles @jimchamp

Copilot

Pull Request Overview

This PR refactors the async follow functionality for list cards to improve user experience by preventing page redirects and adding better internationalization support. The changes focus on making the follow/unfollow actions seamless and adding proper localization.

Key changes:

Added internationalization support for follow/unfollow buttons with proper i18n strings
Implemented button disabling during requests and proper error handling with toast notifications
Refactored code organization by extracting async following logic into a separate method

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
openlibrary/templates/lists/list_follow.html	Added alt attributes for accessibility and moved owner_username variable declaration
openlibrary/plugins/openlibrary/js/book-page-lists.js	Added new initAsyncFollowing method with fetch-based follow/unfollow functionality
openlibrary/macros/Follow.html	Added i18n data attributes to follow buttons for localization support
openlibrary/i18n/messages.pot	Added new translatable strings for accessibility alt texts

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-25T06:52:12Z

openlibrary/plugins/openlibrary/js/book-page-lists.js

+
+                        if (publisherField.value === publisher) {
+                            const followButton = followForm.querySelector('button');
+                            const i18nStrings = JSON.parse(followButton.dataset.i18n)


JSON.parse can throw an error if the dataset contains invalid JSON. This should be wrapped in a try-catch block to handle potential parsing errors gracefully.

openlibrary/plugins/openlibrary/js/book-page-lists.js

jimchamp · 2025-10-01T20:20:17Z

openlibrary/macros/Follow.html

      <input type="hidden" value="$custom_request_path" name="redir_url"/>
      $ css_treatment = 'delete' if following else 'primary'
-      <button type="submit" class="cta-btn cta-btn--$(css_treatment)">$_("Unfollow" if following else "Follow")</button>
+      <button type="submit" class="cta-btn cta-btn--$(css_treatment)" data-i18n="$json_encode(i18n_strings)">$_("Unfollow" if following else "Follow")</button>


Suggested change

<button type="submit" class="cta-btn cta-btn--$(css_treatment)" data-i18n="$json_encode(i18n_strings)">$_("Unfollow" if following else "Follow")</button>

<button type="submit" class="cta-btn cta-btn--$(css_treatment)" data-i18n="$json_encode(i18n_strings)">$(_("Unfollow") if following else _("Follow"))</button>

Somehow, this is creating a "UnfollowFollow" entry in messages.pot, here.

This should probably be fixed in a separate PR. Will open an issue after this is merged.

jimchamp

I've left several suggested code changes. Many of them are not showstoppers, and can likely be handled by new contributors as good first issues (I'll create them after this is merged). However, I cannot merge this until the issues with the initAsyncFollowing function are resolved.

I'm planning on committing the code changes that fix the issues, finding a reviewer for that commit, merging this branch into main, then making some follow-up issues.

jimchamp · 2025-10-01T20:45:58Z

openlibrary/templates/lists/list_follow.html

            <div class="list-follow-card__bottom">
                <div class="list-follow-card__user">
+
+                            $ owner_username = owner.key.split('/')[-1]


Suggested change

$ owner_username = owner.key.split('/')[-1]

$ owner_username = owner.key.split('/')[-1]

openlibrary/macros/Follow.html

jimchamp · 2025-10-01T21:52:39Z

openlibrary/plugins/openlibrary/js/book-page-lists.js

+                        const followForms = listSection.querySelectorAll('.follow-form');
+                        initAsyncFollowing(followForms)


Suggested change

const followForms = listSection.querySelectorAll('.follow-form');

initAsyncFollowing(followForms)

const followForms = listSection.querySelectorAll('.follow-form');

initAsyncFollowing(followForms)

This needs to be here, as we are hydrating follow buttons that were asynchronously rendered.

We should also query the page for and hydrate follow buttons in the main index.js file. Doing so would make these buttons asynchronous throughout the site.

We'd likely also want to move the initAsyncFollowing function to a new file (maybe follows.js) and import it into this file.

jimchamp · 2025-10-01T21:59:14Z

openlibrary/plugins/openlibrary/js/book-page-lists.js

+            followForms.forEach(followForm => {
+                const followButton = followForm.querySelector('button');
+                followButton.disabled = true;
+                followButtonRefs.push(followButton)
+            });


The only button that should be disabled is the one that is in the submitted form. Otherwise, the patron, if following multiple people, would have to wait until one follow request resolves before they can follow another person.

This is also a nested loop on the same collection, which should be avoided.

jimchamp · 2025-10-01T22:08:14Z

openlibrary/plugins/openlibrary/js/book-page-lists.js

+                        throw new Error('Network response was not ok');
+                    }
+
+                    followForms.forEach(followForm => {


Another nested loop on the same collection. We need only change the button that is in the submitted form -- there's no need to update all follow buttons on the page.

jimchamp · 2025-10-01T22:12:18Z

openlibrary/plugins/openlibrary/js/book-page-lists.js

+                    });
+                })
+                .catch(() => {
+                    new PersistentToast('Failed to update followers.  Please try again in a few moments.').show();


Suggested change

new PersistentToast('Failed to update followers. Please try again in a few moments.').show();

new PersistentToast(i18nStrings.errorMsg).show();

Use localized strings when communicating to patrons.

Note: errorMsg must first be added to the i18n dict in follows.html.

openlibrary/plugins/openlibrary/js/book-page-lists.js

for more information, see https://pre-commit.ci

jimchamp · 2025-10-01T23:18:59Z

I'm going to go ahead and merge this one. @cdrini notes that my commit will fail to update multiple follow buttons for a single person (e.g. two follow buttons for patron /people/openlibrary) should they appear on a single page -- only the clicked button would be updated. This isn't ideal, but I don't think that it's a showstopper.

ragipidavid added 6 commits September 24, 2025 23:38

added internationalization

881775c

Disabled follow buttons

6867513

Fixed following bug for user with multiple lists

25f00fa

Added import for persistent toast

2dac08a

Removed unusewd parameter for catch block

060ce9f

added internationalization

009b67e

Copilot AI review requested due to automatic review settings September 25, 2025 06:51

mekarpeles assigned jimchamp Sep 25, 2025

Copilot AI reviewed Sep 25, 2025

View reviewed changes

mekarpeles mentioned this pull request Sep 25, 2025

10945/refactor/refactor-list-design-2 #11239

Closed

cdrini added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label Sep 29, 2025

jimchamp reviewed Oct 1, 2025

View reviewed changes

jimchamp requested changes Oct 1, 2025

View reviewed changes

jimchamp and others added 2 commits October 1, 2025 15:30

Simplify initAsyncFollowing function

873681c

[pre-commit.ci] auto fixes from pre-commit.com hooks

81df69a

for more information, see https://pre-commit.ci

jimchamp approved these changes Oct 1, 2025

View reviewed changes

jimchamp merged commit fa4b9b8 into master Oct 1, 2025
9 checks passed

jimchamp deleted the ragipidavid-10945/refactor/refactor-list-design-2 branch October 1, 2025 23:21

This was referenced Oct 2, 2025

Make all follow buttons work asynchronously #11327

Open

Fix button label translation for "Follow" buttons #11328

Merged

	<button type="submit" class="cta-btn cta-btn--$(css_treatment)" data-i18n="$json_encode(i18n_strings)">$_("Unfollow" if following else "Follow")</button>
	<button type="submit" class="cta-btn cta-btn--$(css_treatment)" data-i18n="$json_encode(i18n_strings)">$(_("Unfollow") if following else _("Follow"))</button>

	$ owner_username = owner.key.split('/')[-1]
	$ owner_username = owner.key.split('/')[-1]

		const followForms = listSection.querySelectorAll('.follow-form');
		initAsyncFollowing(followForms)

	new PersistentToast('Failed to update followers. Please try again in a few moments.').show();
	new PersistentToast(i18nStrings.errorMsg).show();

Uh oh!

List Card Async Follow Functionality #11311

List Card Async Follow Functionality #11311

Uh oh!

Conversation

mekarpeles commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Technical

Testing

Screenshot

Stakeholders

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jimchamp Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

jimchamp left a comment

Choose a reason for hiding this comment

Uh oh!

jimchamp Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jimchamp Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

jimchamp Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

jimchamp Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

jimchamp Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jimchamp commented Oct 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mekarpeles commented Sep 25, 2025 •

edited

Loading