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

Fix: Desktop - Login.gov modal alignment #1598

Merged
merged 1 commit into from
Aug 1, 2023
Merged

Conversation

machikoyasuda
Copy link
Member

@machikoyasuda machikoyasuda commented Aug 1, 2023

fix #1575

What this PR does

This PR ensures that the 2 Login.gov modals match the design in Figma and also match the design of the 2 other modals (Contactless pay and Littlepay) in padding, margin and spacing.

  • Use modal-info includes for both Login.gov modals. (Side note: Why are 2 modal includes necessary? Because spacing and margins are different in seemingly small but noticeable ways between the Transit Provider Selection modal and the rest of the modals. The X icons aren't even the same. The header top padding height is totally different. The titles are differently aligned - to name a few.)
  • Reduces the space between the Header text and the X to 36px.
  • Reduces space between paragraphs

Login.gov modal - Eligibility

image

Login.gov modal - Eligibility Start

image

@machikoyasuda machikoyasuda requested a review from a team as a code owner August 1, 2023 00:29
@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 labels Aug 1, 2023
@machikoyasuda machikoyasuda self-assigned this Aug 1, 2023
@machikoyasuda machikoyasuda added this to the Veterans milestone Aug 1, 2023
@thekaveman
Copy link
Member

thekaveman commented Aug 1, 2023

@machikoyasuda would it make sense to override the other way here?

Since the homepage Provider select modal is the different one, implement the default and override only for the homepage?

@machikoyasuda
Copy link
Member Author

machikoyasuda commented Aug 1, 2023

@thekaveman I asked Sarah about this. She's going to be updating the Agency Provider modal for the SBMTD feature. And when she does that, she is going to align the Agency Provider modal to look more like the Info Modal. So I think we can combine the 2 modals into the same includes once she does that.

image

@machikoyasuda
Copy link
Member Author

@thekaveman Once we get the new Agency Provider modal design (for SBMTD) - which is part of Sprint 8, I suggest we:

  • Consolidate modal includes to 1
  • Fix the X icon. Export the X icon from Figma and use it on that 1 modal includes.

@machikoyasuda machikoyasuda merged commit afc5117 into dev Aug 1, 2023
7 checks passed
@machikoyasuda machikoyasuda deleted the fix/1575-modal-top branch August 1, 2023 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployment-dev [auto] Changes that will trigger a deploy if merged to dev front-end HTML/CSS/JavaScript and Django templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Login.gov modal top padding
2 participants