-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix: migrate registration newsletter to native HTML checkbox #11400
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.
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
base: master
Are you sure you want to change the base?
fix: migrate registration newsletter to native HTML checkbox #11400
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
PTAL @jimchamp, thanks. |
There was a problem hiding this 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 refactors the Internet Archive newsletter checkbox from being a form-rendered input to a hardcoded HTML input in the signup template.
- Removes the
ia_newsletterCheckbox field from theRegisterFormclass - Adds direct HTML checkbox markup for
ia_newsletterin the template - Updates the i18n message location from Python forms file to the HTML template
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| openlibrary/plugins/upstream/forms.py | Removes the ia_newsletter Checkbox field definition from RegisterForm |
| openlibrary/templates/account/create.html | Adds hardcoded HTML checkbox input for ia_newsletter with inline label |
| openlibrary/i18n/messages.pot | Moves the newsletter checkbox text translation entry from forms.py to account/create.html |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <input type="checkbox" id="ia_newsletter" name="ia_newsletter"/> | ||
| <label for="ia_newsletter">$:_('I want to receive news, announcements, and resources from the <a href="https://archive.org/">Internet Archive</a>, the non-profit that runs Open Library.')</label> |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two checkboxes now have inconsistent implementations. The ia_newsletter checkbox (lines 79-80) uses hardcoded HTML, while the pd_request checkbox (line 83) uses form rendering with $:form.pd_request.render(). This inconsistency makes the code harder to maintain. Consider either keeping both as form-rendered checkboxes or converting both to hardcoded HTML for consistency.
| <input type="checkbox" id="ia_newsletter" name="ia_newsletter"/> | |
| <label for="ia_newsletter">$:_('I want to receive news, announcements, and resources from the <a href="https://archive.org/">Internet Archive</a>, the non-profit that runs Open Library.')</label> | |
| $:form.ia_newsletter.render() <label for="ia_newsletter">$:_('I want to receive news, announcements, and resources from the <a href="https://archive.org/">Internet Archive</a>, the non-profit that runs Open Library.')</label> |
|
could anyone please suggest if I can make both checkboxes the same? |
Closes #10731
Changes
Screenshot
Stakeholders
@jimchamp