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

reCAPTCHA: Elig Index, Elig Confirm - Add reCAPTCHA text, hide flag #2586

Open
wants to merge 18 commits into
base: refactor/recaptcha-copy
Choose a base branch
from
Open

reCAPTCHA: Elig Index, Elig Confirm - Add reCAPTCHA text, hide flag #2586

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d81de3e
feat(elig-index): create recaptcha-text, add spanish, add to elig index
machikoyasuda Dec 11, 2024
8a5d4cd
refactor(form): put recaptcha link in form instead
machikoyasuda Dec 11, 2024
32c9297
fix(form): fix alignment of recaptcha, was slightly too wide
machikoyasuda Dec 11, 2024
7a81aab
fix(elig-confirm): form does not need to be wrapped in container/row
machikoyasuda Dec 11, 2024
d331ca6
feat(recaptcha-text): add text styling, add icon size styling
machikoyasuda Dec 11, 2024
129d108
fix(recaptcha-text): don't link the Google part, for En and Es
machikoyasuda Dec 11, 2024
128d28a
feat(recaptcha): hide the badge with CSS
machikoyasuda Dec 11, 2024
29dad70
fix(recaptcha-text): add mt-0 to reset margin-top from bootstrap
machikoyasuda Dec 11, 2024
7479af3
refactor: refactor form to allow for a mb-fix that shortens space und…
machikoyasuda Dec 11, 2024
5d34bb4
fix: update comment to proper classname
machikoyasuda Dec 11, 2024
9cfd2c6
refactor(css): use a footer style rather than negative margin hack
machikoyasuda Dec 11, 2024
a4e9b6f
refactor(css): variable-ize external link icon size
machikoyasuda Dec 16, 2024
1f2ee3e
fix(css): contain icon size
machikoyasuda Dec 16, 2024
87f40f7
docs: add clarifying comments
machikoyasuda Dec 17, 2024
b21f806
fix(recaptcha): use display none on badge
machikoyasuda Dec 17, 2024
986fc6f
fix(css): remove 24px footer margin from elig-confirm
machikoyasuda Dec 17, 2024
ec05326
fix: remove unnecessary class
machikoyasuda Dec 17, 2024
dce769c
fix(form): undo form refactoring for now
machikoyasuda Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 30 additions & 31 deletions benefits/core/templates/core/includes/form.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,41 +3,39 @@

{% url form.action_url as form_action %}

<form id="{{ form.id }}" action="{{ form_action }}" method="{{ form.method | default:"post" | upper }}" role="form">
<form id="{{ form.id }}"
class="col-lg-6 {{ form.classes }}"
action="{{ form_action }}"
method="{{ form.method | default:"post" | upper }}"
role="form">
{% csrf_token %}

<div class="row form-container justify-content-center">
<div class="{{ form.classes }}">
Copy link
Member

Choose a reason for hiding this comment

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

I think there's a regression in Benefits Admin coming from this change.

We have some styles in admin/styles.css so that descendant checkboxes will be on the left side:

benefits/benefits/static/css/admin/styles.css

Lines 88 to 103 in dce769c

/* Eligibility Page */
.checkbox-parent .form-group:last-of-type .col-12 {
display: flex;
flex-direction: row-reverse;
justify-content: start;
column-gap: 0.5rem;
margin-top: 2rem;
}
.checkbox-parent,
.checkbox-parent .form-group .col-12,
.checkbox-parent .form-group .col-12 #id_flow {
display: flex;
flex-direction: column;
row-gap: 1rem;
}

InPersonEligibilityForm has checkbox-parent in its classes, which would end up on this line, but now it's on the <form> element.

main this branch (with #2555 merged in locally... *)
image image

Not sure if we should put this <div> back or try to tweak the CSS?

(*When we're back in the new year, it'd be good to get all this reCAPTCHA work caught up with the fixes that are in main.)

{% for field in form %}
<div class="row form-group mb-0">
<div class="col-12">
{# djlint:off #}
{% if field.label %}
<label for="{{ field.id_for_label }}" class="form-control-label">{{ field.label }}{% if field.field.required %}<span class="required-label text-body">*</span>{% endif %}
</label>
{% endif %}
{# djlint:on #}

{{ field }}

{% if field.help_text %}<small class="d-block mt-2 pt-1 form-text text-body">{{ field.help_text }}</small>{% endif %}
</div>
</div>
{% endfor %}
</div>
Comment on lines -9 to -27
Copy link
Member Author

Choose a reason for hiding this comment

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

All of these row justify-content-center, row, col-12 are unnecessary.

<div class="form-field-container d-flex flex-column gap-4">
{% for field in form %}
<div class="form-group mb-0">
{# djlint:off #}
{% if field.label %}
<label for="{{ field.id_for_label }}" class="form-control-label">{{ field.label }}{% if field.field.required %}<span class="required-label text-body">*</span>{% endif %}
</label>
{% endif %}
{# djlint:on #}

{{ field }}

{% if field.help_text %}<small class="d-block mt-2 pt-1 form-text text-body">{{ field.help_text }}</small>{% endif %}
</div>
{% endfor %}
</div>

{% if form.submit_value %}
<div class="row d-flex justify-content-center pt-8">
<div class="col-lg-6">
Comment on lines -31 to -32
Copy link
Member Author

Choose a reason for hiding this comment

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

Now longer needed.

<button class="btn btn-lg btn-primary spinner-hidden d-flex justify-content-center align-items-center"
data-action="submit"
type="submit"
form="{{ form.id }}">
<span class="btn-text">{{ form.submit_value }}</span>
<span class="spinner-border spinner-border-sm"></span>
</button>
</div>
<div class="pt-8">
<button class="btn btn-lg btn-primary spinner-hidden d-flex justify-content-center align-items-center"
data-action="submit"
type="submit"
form="{{ form.id }}">
<span class="btn-text">{{ form.submit_value }}</span>
<span class="spinner-border spinner-border-sm"></span>
</button>
</div>
{% endif %}

Expand Down Expand Up @@ -94,6 +92,8 @@
{% endcomment %}
machikoyasuda marked this conversation as resolved.
Show resolved Hide resolved
<input type="hidden" name="{{ request.recaptcha.data_field }}" value="">

<div class="pt-8">{% include "core/includes/recaptcha-text.html" %}</div>

<script nonce="{{ request.csp_nonce }}">
function recaptchaSubmit($event) {
// checks the validity of the form. Return if invalid; HTML5 validation errors should display
Expand Down Expand Up @@ -121,6 +121,5 @@
$("button[type=submit]", "#{{ form.id }}").on("click", recaptchaSubmit);
</script>
{% endif %}

</form>
{% endif %}
7 changes: 7 additions & 0 deletions benefits/core/templates/core/includes/recaptcha-text.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{% load i18n %}

<p class="form-text text-body mt-0">
{% blocktranslate trimmed %}
This site is protected by reCAPTCHA and the Google <a href="https://policies.google.com/privacy" target="_blank" rel="noopener noreferrer">Privacy Policy</a> and <a href="https://policies.google.com/terms" target="_blank" rel="noopener noreferrer">Terms of Service</a> apply.
{% endblocktranslate %}
</p>
2 changes: 0 additions & 2 deletions benefits/eligibility/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ def __init__(self, agency: models.TransitAgency, *args, **kwargs):
super().__init__(*args, **kwargs)
flows = agency.enrollment_flows.filter(supported_enrollment_methods__contains=models.EnrollmentMethods.DIGITAL)

self.classes = "col-lg-6"
# second element is not used since we render the whole label using selection_label_template,
# therefore set to None
flow_field = self.fields["flow"]
Expand Down Expand Up @@ -110,7 +109,6 @@ def __init__(
self.headline = headline
self.blurb = blurb

self.classes = "col-lg-6"
sub_widget = widgets.FormControlTextInput(placeholder=sub_placeholder)
if sub_pattern:
sub_widget.attrs.update({"pattern": sub_pattern})
Expand Down
12 changes: 9 additions & 3 deletions benefits/eligibility/templates/eligibility/confirm.html
View file
Open in desktop
angela-tran marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
{% extends "core/base.html" %}

{% block classes %}
{{ block.super | add:" eligibility-confirm" }}
{% endblock classes %}

{% block page-title %}
{{ form.title }}
{% endblock page-title %}
Expand All @@ -26,7 +30,9 @@ <h1>{{ form.headline }}</h1>
{% endblock explanatory-text-wrapper %}

{% block inner-content %}
<div class="container">
<div class="row">{% include "core/includes/form.html" with form=form %}</div>
</div>
{% include "core/includes/form.html" with form=form %}
{% endblock inner-content %}

{% comment %} Override with empty block to prevent rendering empty divs {% endcomment %}
{% block call-to-action %}
{% endblock call-to-action %}
4 changes: 4 additions & 0 deletions benefits/eligibility/templates/eligibility/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,7 @@ <h1>{% translate "Choose the transit benefit you would like to enroll in" %}</h1
});
</script>
{% endblock inner-content %}

{% comment %} Override with empty block to prevent rendering empty divs {% endcomment %}
{% block call-to-action %}
{% endblock call-to-action %}
10 changes: 9 additions & 1 deletion benefits/locale/en/LC_MESSAGES/django.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
msgid ""
msgstr ""
"Report-Msgid-Bugs-To: https://github.com/cal-itp/benefits/issues \n"
"POT-Creation-Date: 2024-12-09 13:27-0800\n"
"POT-Creation-Date: 2024-12-11 12:16-0800\n"
"Language: English\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
Expand Down Expand Up @@ -320,6 +320,14 @@ msgid ""
"JavaScript. Please enable JavaScript for this website and"
msgstr ""

msgid ""
"This site is protected by reCAPTCHA and the Google <a href=\"https://"
"policies.google.com/privacy\" target=\"_blank\" rel=\"noopener "
"noreferrer\">Privacy Policy</a> and <a href=\"https://policies.google.com/"
"terms\" target=\"_blank\" rel=\"noopener noreferrer\">Terms of Service</a> "
"apply."
msgstr ""

msgid "Get a reduced fare on CST public transportation when you tap to ride"
msgstr ""

Expand Down
15 changes: 14 additions & 1 deletion benefits/locale/es/LC_MESSAGES/django.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
msgid ""
msgstr ""
"Report-Msgid-Bugs-To: https://github.com/cal-itp/benefits/issues \n"
"POT-Creation-Date: 2024-12-09 13:27-0800\n"
"POT-Creation-Date: 2024-12-11 12:16-0800\n"
"Language: Español\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
Expand Down Expand Up @@ -434,6 +434,19 @@ msgstr ""
"Para funcionar correctamente, este sitio web requiere un navegador que "
"admita JavaScript. Por favor, active JavaScript por este sitio web y"

msgid ""
"This site is protected by reCAPTCHA and the Google <a href=\"https://"
"policies.google.com/privacy\" target=\"_blank\" rel=\"noopener "
"noreferrer\">Privacy Policy</a> and <a href=\"https://policies.google.com/"
"terms\" target=\"_blank\" rel=\"noopener noreferrer\">Terms of Service</a> "
"apply."
msgstr ""
"Este sitio está protegido por reCAPTCHA y se aplican la <a href=\"https://"
"policies.google.com/privacy\" target=\"_blank\" rel=\"noopener "
"noreferrer\">Política de privacidad</a> y <a href=\"https://policies.google."
"com/terms\" target=\"_blank\" rel=\"noopener noreferrer\">las Condiciones de "
"servicio</a> de Google."

msgid "Get a reduced fare on CST public transportation when you tap to ride"
msgstr ""
"Obtenga una tarifa reducida en el transporte público CST cuando toque para "
Expand Down
49 changes: 32 additions & 17 deletions benefits/static/css/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ html {
scroll-padding-top: 101px; /* Header Height (80px) + Skip Nav Height (21px) = 101px */
}

/* reCAPTCHA: Hide the badge */
.grecaptcha-badge {
visibility: hidden;
thekaveman marked this conversation as resolved.
Show resolved Hide resolved
}

body {
font-size: 100%;
color: var(--text-primary-color);
Expand Down Expand Up @@ -53,6 +58,8 @@ li {
margin: 0;
}

/* Utility Classes */

@media (min-width: 992px) {
/* Utility Class: Padding bottom 64px on Desktop */
/* Pair with pb-4 on Form pages to get 24px on Mobile, 64px on Desktop */
Expand Down Expand Up @@ -106,6 +113,11 @@ a:visited:not(.btn) {
color: var(--selected-color);
}

:root {
--external-link-icon-size: var(--font-size-24px);
--external-link-icon-size-small: var(--font-size-18px);
}

a[target="_blank"]::after {
content: " ";
background-color: currentColor;
Expand All @@ -119,12 +131,19 @@ a[target="_blank"]::after {

display: inline-block;
position: relative;
width: 24px;
height: 24px;
width: var(--external-link-icon-size);
height: var(--external-link-icon-size);
margin-left: 2px;
vertical-align: text-bottom;
}

.form-text a[target="_blank"]::after {
width: var(--external-link-icon-size-small);
height: var(--external-link-icon-size-small);
mask-size: contain;
-webkit-mask-size: contain;
}

/* Headlines */
/* All headlines */
h1,
Expand Down Expand Up @@ -285,6 +304,10 @@ footer {
margin-top: 64px;
}

.eligibility-confirm footer {
margin-top: 24px;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

image

The Elig Index page's reCAPTCHA text has 64px padding both above and below the text, but the Elig Confirm's has 64px above and 24px below. So this class allows us to get that. This is the only page on the whole app, aside from the Index page, to not have that 64px buffer above the footer.

Copy link
Member

@angela-tran angela-tran Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I narrowed down the issue I mentioned in #2586 (comment) for confirm.html to this style... somehow this is causing the footer to have spacing on the bottom?

Screenshot from 2024-12-17 13-51-25

Screenshot from 2024-12-17 13-51-43

footer .footer-links li a.footer-link {
color: var(--footer-link-color);
font-weight: var(--footer-link-font-weight);
Expand Down Expand Up @@ -513,26 +536,18 @@ footer .footer-links li a.footer-link:visited {
}

/* Forms: Text Inputs */
/* Form Container: Must use .form-container parent to use these styles */

:root {
--form-input-gap: calc(24rem / 16);
}

.form-container .form-group:not(:first-child) {
padding-top: var(--form-input-gap);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Use the d-flex flex-column gap-4 CSS class to achieve this instead.

/* Form Field Container: Must use .form-field-container parent to use these styles */
Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed this b/c this div is a container around one form field, rather than the entire form.


.form-container .form-control-label,
.form-container .required-label {
.form-field-container .form-control-label,
.form-field-container .required-label {
font-size: var(--font-size-16px);
font-weight: var(--medium-font-weight);
line-height: var(--form-field-text-line-height);
letter-spacing: calc(var(--font-size-16px) * var(--letter-spacing-5));
padding-bottom: calc(12rem / 16);
}

.form-container .form-control {
.form-field-container .form-control {
border-width: var(--border-width);
border-radius: var(--border-radius);
border-color: var(--primary-color);
Expand All @@ -542,8 +557,8 @@ footer .footer-links li a.footer-link:visited {
/* Input height should be 60px */
}

.form-container .form-control:focus,
.form-container .form-control:focus-visible {
.form-field-container .form-control:focus,
.form-field-container .form-control:focus-visible {
outline: 3px solid var(--focus-color) !important;
outline-offset: 2px !important;
box-shadow: none !important;
Expand All @@ -552,7 +567,7 @@ footer .footer-links li a.footer-link:visited {
.form-text {
font-size: var(--font-size-14px);
line-height: var(--form-field-text-line-height);
letter-spacing: var(--letter-spacing-5);
letter-spacing: calc(var(--font-size-14px) * var(--letter-spacing-5));
Copy link
Member Author

Choose a reason for hiding this comment

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

image This letter-spacing update affects not just reCAPTCHA text, but the form field helper text as well. It was not updated on Production.

The way this is calculated: In Figma, click on the text. In the Dev resources sidebar, ensure Properties is enabled (instead of Code). Look for the letter spacing percentage:
image

Multiply the percentage with the font size.

}

/* Forms: Radio Buttons */
Expand Down
Loading