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.

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

LittlePay Modal #1516

Merged
merged 11 commits into from Jul 12, 2023
Merged

LittlePay Modal #1516

merged 11 commits into from Jul 12, 2023

Conversation

machikoyasuda
Copy link
Member

@machikoyasuda machikoyasuda commented Jul 11, 2023

closes #1481

What this PR does

  • Enrollment Index: Add LittlePay link modal
  • Modal Trigger Link: Add CSS class that creates modal trigger link icon style. Use CSS class in Modal Trigger includes. Changes Trigger inclues to allow CSS classes as a string to be passed in. The Modal Trigger Link is composed of a button, but has the styles of a default text link
  • Info Modal includes: Add an InfoModal template, which has a slightly different style from the Agency Selector modal

Documentation

Modal Trigger Link

  • Takes: id, classes, text
  • Use the CSS class "modal-trigger--link" for an inline link (but actually button on the inside) with the (?) around it. (The reason why it has to be a button is so that the modal opens when focused and a user clicks spacebar or the enter key).
  • Note: This PR does not yet implement the new Update focus ring  #1387 style color to yellow. That has to happen all together for all links. The spacing, however, is ready to fit the link and icon together.
image image image

Modal Info

  • Takes: size - modal-lg or modal-md (classes from Bootstrap)
  • The 4 info modals have different styles than the Agency Selector modal. So I created an includes called "modal-info" (taking name suggestions!) that has the different styles for the header height, title font and padding.
  • This includes also takes a size class, either modal-lg or modal-md

Modal

image image image image image

@machikoyasuda machikoyasuda requested a review from a team as a code owner July 11, 2023 17:22
@github-actions github-actions bot added front-end HTML/CSS/JavaScript and Django templates deployment-dev [auto] Changes that will trigger a deploy if merged to dev i18n Copy: Language files or Django i18n framework and removed deployment-dev [auto] Changes that will trigger a deploy if merged to dev labels Jul 11, 2023
@machikoyasuda machikoyasuda self-assigned this Jul 11, 2023
@machikoyasuda machikoyasuda added this to the Veterans milestone Jul 11, 2023
angela-tran
angela-tran previously approved these changes Jul 11, 2023
Copy link
Member

@angela-tran angela-tran left a comment

Choose a reason for hiding this comment

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

The CSS here looks good, and I'm glad to see the refactor from #1504 was helpful here!

My only comment is that I think modal-info.html and modal.html are similar enough to where they could be one file.

</div>
</div>
</div>
{% endwith %}
Copy link
Member

Choose a reason for hiding this comment

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

This code is pretty similar to modal.html. The only differences are size on line 4 and some spacing (p-2 on line 6, pt-0 mt-0 p-4 m-2 on line 9).

It might be worth it to find a way to make them one file. It doesn't necessarily have to be in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely can be done in a future PR!

@machikoyasuda machikoyasuda merged commit 54a4c1f into dev Jul 12, 2023
11 checks passed
@machikoyasuda machikoyasuda deleted the feat/1481-veterans--littlepay-modal branch July 12, 2023 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
front-end HTML/CSS/JavaScript and Django templates i18n Copy: Language files or Django i18n framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enrollment Index - Modal for Littlepay (All flows)
2 participants