Eligibility Start: Contactless Pay Modal #1523

machikoyasuda · 2023-07-12T03:38:00Z

closes #1475

What this PR does

Create Contactless modal
Add Contactless modal trigger
Bug: Fixes CSS bug on mobile
Refactor: Refactor all the modals that are triggered by text (rather than a button or an image, like the first Login.gov modal) - the Eligibility Start/Login.gov modal, Contactless Pay modal and LittlePay modal.
Fix: Add Go back button to the all informational modals on mobile

How to test

Desktop: Make sure to open modal by clicking and also, keyboard tab focus + enter
Mobile: Open, view whole modal and close modal
Make sure all info modals have Back link and that the link works

Screenshots

Mobile: Trigger

Note: Now that the LittlePay modal is also a text modal, there is a chance that the (?) breaks into a second line, on narrower devices.

Mobile: Modal

Desktop: Trigger

Desktop: Modal - English

Desktop: Modal - Spanish

Mobile: Modal - Eng and Span with Go back button

thekaveman · 2023-07-14T16:34:41Z

@machikoyasuda I rebased this on dev because we merged #1533 which renamed a bunch of template files and caused other conflicts here. Hopefully I didn't clobber any of your work!

benefits/core/templates/core/includes/modal-trigger-link.html

thekaveman · 2023-07-20T17:31:51Z

benefits/eligibility/templates/eligibility/includes/media-item--bankcardcheck--start.html

+        <p>
+            {% translate "eligibility.pages.start.bankcard.text" %}
+            {% translate "eligibility.pages.start.modal.title" as trigger_text %}
+            {% include "core/includes/modal-trigger-link.html" with id="contactless-modal" classes="modal-trigger--link" text=trigger_text %}


Seems like this modal-trigger--link class is applied to each, could it be moved to the common include?

It could, but in general, I want to keep the includes more flexible and less restrictive - so I opted to keep the CSS class out of the includes.

I think the fact that this specific include is the modal trigger link... like, we always want that class associated with that include right? It isn't a more generic include that is used in different ways.

Definitely agree with you in general to try and keep them more flexible.

Decided to take a different CSS refactor approach here: 04bb644#diff-744df555aee38173a0b0868baa7befa720e6a28f80747523e74e724c126cd381R708-R729

Similar to how @angela-tran's external link CSS selector grabs all links, the code that adds the question mark icon grabs all links with [data-bs-toggle="modal"]:not(.btn) - that is, all modal triggers that are not .btn style buttons. All the modal triggers besides the Agency Selector have the question mark.

The one modal trigger that needs additional styles is the Login.gov icon button - so I added those as CSS Bootstrap utility classes.

benefits/eligibility/templates/eligibility/includes/modal--contactless.html

benefits/locale/es/LC_MESSAGES/django.po

machikoyasuda · 2023-07-20T18:34:36Z

benefits/eligibility/templates/eligibility/includes/media-item--idcardcheck--start--senior.html

-            <!-- Button to trigger modal -->
-            <button type="button" class="modal-trigger--link" data-bs-toggle="modal" data-bs-target="#identity-verification-help">
-                {% translate "eligibility.pages.start.senior.help.link_text" %}
-            </button>
+            {% translate "eligibility.pages.start.senior.help.link_text" as trigger_text %}
+            {% include "core/includes/modal-trigger-link.html" with id="identity-verification-help" classes="modal-trigger--link" text=trigger_text %}


Interesting thing I learned: The above red code produces this tiny extra space between the end of the word
verification and the (?)

By passing in the trigger text into the includes instead, the mysterious extra space is removed.

I think something about the new line might be adding a space or something. Not sure!

…t styles

machikoyasuda · 2023-07-20T19:05:25Z

benefits/static/css/styles.css

-.modal-trigger--link {
-  background: transparent;
-  border: none;
-  color: var(--primary-color);
-  font-family: var(--bs-body-font-family);
-  font-weight: var(--bold-font-weight);
-  letter-spacing: var(--body-letter-spacing);
-  padding: 0;
-  margin-right: -5px;
-  text-decoration: underline;
-  text-decoration-style: solid;
-}


These are no longer necessary b/c we're using an <a> element for the modal trigger. These classes were only overriding <button> defaults and adding <a> defaults.

…lect style logic

angela-tran

This looks good to me 👍

machikoyasuda self-assigned this Jul 12, 2023

machikoyasuda added this to the Veterans milestone Jul 12, 2023

github-actions bot added deployment-dev [auto] Changes that will trigger a deploy if merged to dev front-end HTML/CSS/JavaScript and Django templates i18n Copy: Language files or Django i18n framework labels Jul 12, 2023

thekaveman mentioned this pull request Jul 14, 2023

Fix: template and msgid naming #1533

Merged

thekaveman force-pushed the feat/1475-veterans--modal-contactless branch from bda411e to 81d90b8 Compare July 14, 2023 16:33

thekaveman force-pushed the feat/1475-veterans--modal-contactless branch from 81d90b8 to 48364c8 Compare July 14, 2023 16:39

machikoyasuda force-pushed the feat/1475-veterans--modal-contactless branch from 48364c8 to 934e683 Compare July 18, 2023 19:09

machikoyasuda added 4 commits July 19, 2023 18:49

feat(eligibility-start): first pass at contactless modal

6d9ef1e

fix(eligibility-start): modal - use msgid for trigger text

146d286

chore: rebase dev fix

25d27dc

fix(copy): fix duplicate copy

72bbe44

machikoyasuda force-pushed the feat/1475-veterans--modal-contactless branch from b8c6bb4 to 72bbe44 Compare July 19, 2023 19:02

machikoyasuda added 6 commits July 19, 2023 20:34

fix(modal-contactless): add the contact info line

6503e20

fix(elig-start): remove extra white space

958008a

fix(elig-start): undo the underline span

b51cbf6

fix(modal-trigger): create Modal Trigger Link for inline-link modals

955ce44

refactor(enrollment-index): use Modal Trigger Link for LittlePay modal

9e10616

refactor(css): unnecessary declaration

d12b83e

machikoyasuda marked this pull request as ready for review July 19, 2023 22:58

machikoyasuda requested a review from a team as a code owner July 19, 2023 22:58

machikoyasuda added 2 commits July 19, 2023 23:29

fix(modals): add Go back link on modals on mobile

4226bbe

fix(modal): add Go back to littlepay modal on mobile too

24843f1

thekaveman reviewed Jul 20, 2023

View reviewed changes

machikoyasuda commented Jul 20, 2023

View reviewed changes

machikoyasuda added 3 commits July 20, 2023 18:43

refactor: remove unnecessary css

00fa20f

refactor(modal): add Go back modal link text to Django.po files

3bc0e20

fix(eligibility-start): use bootstrap classes to remove button defaul…

f9ddec6

…t styles

machikoyasuda commented Jul 20, 2023

View reviewed changes

machikoyasuda added 2 commits July 20, 2023 19:33

refactor(css): refactor modal trigger link css selector to better ref…

04bb644

…lect style logic

fix(css): p-0 not padding-0

0ff5504

machikoyasuda requested a review from thekaveman July 20, 2023 19:50

chore: add back gh issue link

9aa49b0

angela-tran approved these changes Jul 20, 2023

View reviewed changes

thekaveman approved these changes Jul 21, 2023

View reviewed changes

machikoyasuda merged commit caae08c into dev Jul 21, 2023

machikoyasuda deleted the feat/1475-veterans--modal-contactless branch July 21, 2023 16:18

angela-tran mentioned this pull request Jul 24, 2023

Chore: Help page - contactless cards section #1558

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eligibility Start: Contactless Pay Modal #1523

Eligibility Start: Contactless Pay Modal #1523

machikoyasuda commented Jul 12, 2023 •

edited

Loading

thekaveman commented Jul 14, 2023

thekaveman Jul 20, 2023

machikoyasuda Jul 20, 2023

thekaveman Jul 20, 2023

machikoyasuda Jul 20, 2023 •

edited

Loading

machikoyasuda Jul 20, 2023

machikoyasuda Jul 20, 2023

angela-tran left a comment

Eligibility Start: Contactless Pay Modal #1523

Eligibility Start: Contactless Pay Modal #1523

Conversation

machikoyasuda commented Jul 12, 2023 • edited Loading

What this PR does

How to test

Screenshots

Mobile: Trigger

Mobile: Modal

Desktop: Trigger

Desktop: Modal - English

Desktop: Modal - Spanish

Mobile: Modal - Eng and Span with Go back button

thekaveman commented Jul 14, 2023

thekaveman Jul 20, 2023

Choose a reason for hiding this comment

machikoyasuda Jul 20, 2023

Choose a reason for hiding this comment

thekaveman Jul 20, 2023

Choose a reason for hiding this comment

machikoyasuda Jul 20, 2023 • edited Loading

Choose a reason for hiding this comment

machikoyasuda Jul 20, 2023

Choose a reason for hiding this comment

machikoyasuda Jul 20, 2023

Choose a reason for hiding this comment

angela-tran left a comment

Choose a reason for hiding this comment

machikoyasuda commented Jul 12, 2023 •

edited

Loading

machikoyasuda Jul 20, 2023 •

edited

Loading