reCAPTCHA: Enrollment index #2575

machikoyasuda · 2024-12-06T19:32:32Z

closes #2546

What this PR does

Media Items: All media item CSS is now gone! See more details here: https://github.com/cal-itp/benefits/pull/2575/files#r1873914833
Media Items: Refactors the 2 items for Enrollment index (Contactless card item, CalFresh item) to not be a <ul>, just a regular headline and paragraph. Then, moves that code directly into the index template directly - removing the includes entirely.
Copy: The headlines previously had manual <br> in them. I removed it.

Test

Test the CalFresh flow
Test the other flows

github-actions · 2024-12-06T19:33:36Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
benefits/eligibility
forms.py
Project Total

_{This report was generated by python-coverage-comment-action}

machikoyasuda · 2024-12-06T19:56:24Z

benefits/core/templates/core/base.html

          {% block call-to-action %}
-            <div class="row d-flex justify-content-lg-end pt-8">
-              <div class="col-lg-3 offset-2 offset-sm-2 offset-lg-0 col-sm-8 col-8">
+            <div class="row d-flex justify-content-center pt-8">
+              <div class="col-12 col-lg-6">
                {% block call-to-action-button %}
                {% endblock call-to-action-button %}


This block is only used by enrollment-index.

Note for #2540

Hmm interesting observation 👍 Sounds like you're leaving any potential clean-up on this to be done in #2540?

Yeah and this is turning into a true PR rabbit hole, but in the next PR for Enrollment Success/Logged Out, https://github.com/cal-itp/benefits/pull/2579/files#r1876743474, I moved some code out of call-to-action to clean it up further. After https://github.com/cal-itp/benefits/pull/2579/files#r1876743474, in #2540 will be able to consolidate some of the blocks in base.

machikoyasuda · 2024-12-06T19:59:49Z

benefits/enrollment/templates/enrollment/index.html

+  <div class="col-12 col-lg-6">
+    {% block media-items %}
+      <ul class="d-flex list-unstyled ps-0 mb-0">
        {% include "enrollment/includes/media-item--contactless-card--index.html" %}
      </ul>
-    </div>
-  {% endblock media-items %}
-  {% block additional-info %}
-  {% endblock additional-info %}
+    {% endblock media-items %}
+    {% block additional-info %}
+    {% endblock additional-info %}
+  </div>


This eliminates some divitis:

Instead of having three <div class="row justify-content-center"><div class="col-lg-6"></div></div>, we just have 1.

machikoyasuda · 2024-12-06T20:02:08Z

benefits/static/css/styles.css

-/* Media List */
-
-:root {
-  --media-item-icon-size: calc(64rem / 16);
-  --media-item-icon-margin: calc(24rem / 16);
-}
-
-@media (min-width: 992px) {
-  :root {
-    --media-item-icon-size: calc(90rem / 16);
-    --media-item-icon-margin: calc(32rem / 16);
-  }
-}
-
-.media-list-icon-left-margin {
-  margin-left: calc(
-    var(--media-item-icon-size) + var(--media-item-icon-margin)
-  );
-}
-


There are officially zero Media Item classes now. It's just a single <ul> with some <li>s in them, where there's always a <h2> and some <p>s.

Media items are used on two pages:

Eligibility Start - There are 2 media items per flow. The content is semantically and stylistically a list. There is no "media" though, so it could be renamed to heading-list or something.

Enrollment Index - There is only 1 item, so it's not even really a list. On this page it really feels like a stretch to semantically even use a ul and li. Just look at it:

Should this PR refactor Enrollment Index so it doesn't use the media-item block and just has a <h2> and <p> in inner-content? @angela-tran

@thekaveman jinx!

Media item now removed from Enrollment Index.

Should this PR refactor Enrollment Index so it doesn't use the media-item block and just has a <h2> and <p> in inner-content? @angela-tran

(this question was essentially the same as the one in #2575 (comment))

Done by @machikoyasuda in 56b9322

machikoyasuda · 2024-12-06T20:03:42Z

benefits/locale/en/LC_MESSAGES/django.po


-msgid "We found your record!<br>Now let’s enroll your contactless card."
+msgid "We found your record! Now let’s enroll your contactless card."
 msgstr ""

 msgid "Eligibility confirmation"
 msgstr ""

-msgid "Your eligibility is confirmed!<br>You’re almost there."
+msgid "Your eligibility is confirmed! You’re almost there."
 msgstr ""


The copy is now free of any headlines with manual <br> breaks in them.

machikoyasuda · 2024-12-06T20:51:33Z

benefits/enrollment/templates/enrollment/index--calfresh.html

+{% block info %}
+  {% include "enrollment/includes/info--contactless-card--index--calfresh.html" %}
+  {% include "enrollment/includes/alert-box--warning--calfresh.html" %}
+{% endblock info %}


Taking suggestions for this naming. @thekaveman @angela-tran Really wasn't sure what to name the block (info) or the includes file name. I could even get rid of the includes and just copy/paste the h2/p/modal in directly. Thoughts?

I could even get rid of the includes and just copy/paste the h2/p/modal in directly.

I vote for this one

Done by @machikoyasuda in 56b9322

…e cta to col-lg-6 width

…emove unused css

…l/li

…ly into template

machikoyasuda · 2024-12-09T21:39:40Z

@angela-tran Now ready for review! Incorporated your feedback on the media item template.

angela-tran

These changes look good to me.

Idk if you want to do this in a separate PR or as a part of this one, but now that Enrollment Index doesn't use any templates that inherit from core/includes/media-item.html, I think we could rename all the media-item templates to say eligibility-item instead (since they're only used in Eligibility Start).

angela-tran · 2024-12-09T22:43:16Z

benefits/core/templates/core/base.html

          {% block call-to-action %}
-            <div class="row d-flex justify-content-lg-end pt-8">
-              <div class="col-lg-3 offset-2 offset-sm-2 offset-lg-0 col-sm-8 col-8">
+            <div class="row d-flex justify-content-center pt-8">
+              <div class="col-12 col-lg-6">
                {% block call-to-action-button %}
                {% endblock call-to-action-button %}


Hmm interesting observation 👍 Sounds like you're leaving any potential clean-up on this to be done in #2540?

machikoyasuda · 2024-12-10T00:40:25Z

@angela-tran Thank you Angela. I will do the renaming in #2540

github-actions bot added front-end HTML/CSS/JavaScript and Django templates i18n Copy: Language files or Django i18n framework and removed front-end HTML/CSS/JavaScript and Django templates labels Dec 6, 2024

machikoyasuda self-assigned this Dec 6, 2024

machikoyasuda linked an issue Dec 6, 2024 that may be closed by this pull request

reCAPTCHA: Enrollment index - Realign text, button #2546

Closed

machikoyasuda commented Dec 6, 2024

View reviewed changes

machikoyasuda mentioned this pull request Dec 6, 2024

reCAPTCHA: Eligibility Start #2573

Merged

machikoyasuda commented Dec 6, 2024

View reviewed changes

Base automatically changed from feat/2542-recaptcha-elig-start to refactor/recaptcha-copy December 9, 2024 18:50

machikoyasuda added 5 commits December 9, 2024 12:59

feat(copy): remove <br> from headings

32d41d4

feat(enrollment-index): use col-lg-6 for heading, content and set bas…

f2ca459

…e cta to col-lg-6 width

feat(enrollment-index-calfresh): realign calfresh enrollment index, r…

2eb8197

…emove unused css

fix(enrollment-index-agency-card): remove large padding bottom

d559c19

refactor(media-item): rename media-item to info, remove unnecessary u…

e187a60

…l/li

machikoyasuda force-pushed the feat/2546-recaptcha-enrollment branch from c6d5503 to e187a60 Compare December 9, 2024 20:59

refactor(media-item): remove template entirely, render content direct…

56b9322

…ly into template

machikoyasuda force-pushed the feat/2546-recaptcha-enrollment branch from a744ac8 to 56b9322 Compare December 9, 2024 21:31

machikoyasuda marked this pull request as ready for review December 9, 2024 21:39

machikoyasuda requested a review from a team as a code owner December 9, 2024 21:39

angela-tran approved these changes Dec 9, 2024

View reviewed changes

machikoyasuda merged commit aab7c0c into refactor/recaptcha-copy Dec 10, 2024
8 checks passed

machikoyasuda deleted the feat/2546-recaptcha-enrollment branch December 10, 2024 00:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reCAPTCHA: Enrollment index #2575

reCAPTCHA: Enrollment index #2575

machikoyasuda commented Dec 6, 2024 •

edited

Loading

github-actions bot commented Dec 6, 2024 •

edited

Loading

machikoyasuda Dec 6, 2024

machikoyasuda Dec 9, 2024

angela-tran Dec 9, 2024

machikoyasuda Dec 9, 2024

machikoyasuda Dec 6, 2024

machikoyasuda Dec 6, 2024 •

edited

Loading

machikoyasuda Dec 6, 2024

machikoyasuda Dec 6, 2024

angela-tran Dec 9, 2024

machikoyasuda Dec 6, 2024

machikoyasuda Dec 6, 2024 •

edited

Loading

angela-tran Dec 9, 2024

angela-tran Dec 9, 2024

machikoyasuda commented Dec 9, 2024

angela-tran left a comment

angela-tran Dec 9, 2024

machikoyasuda commented Dec 10, 2024

reCAPTCHA: Enrollment index #2575

reCAPTCHA: Enrollment index #2575

Conversation

machikoyasuda commented Dec 6, 2024 • edited Loading

What this PR does

Test

github-actions bot commented Dec 6, 2024 • edited Loading

Coverage report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machikoyasuda Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machikoyasuda Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machikoyasuda commented Dec 9, 2024

angela-tran left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machikoyasuda commented Dec 10, 2024

machikoyasuda commented Dec 6, 2024 •

edited

Loading

github-actions bot commented Dec 6, 2024 •

edited

Loading

machikoyasuda Dec 6, 2024 •

edited

Loading

machikoyasuda Dec 6, 2024 •

edited

Loading