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: Eligibility Confirm - Unnest explanatory-text-wrapper div #2277

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

machikoyasuda
Copy link
Member

closes #2271

The problem: Vertical alignment of the We use the ... paragraph is off for both Desktop and Tablet.
image

The issue:
image

These two divs need to be siblings, not a parent/child relationship. Having a row justify-content-center inside another row justify-content-center is giving the paragraph in the child div a more padding left and right. The heading text and the paragraph text need to be siblings, not a parent/child.

This parent/child relationship was created in the base.html file that the confirm.html is based off of.

Since explanatory-text-wrapper is only used on this confirm.html page, I was able to safely change the base.html to fix the issue, and know that this is will only change the confirm page and not any other pages.

Test is by looking at all pages for other regressions, and testing the Confirm page in desktop and mobile

@machikoyasuda machikoyasuda self-assigned this Aug 6, 2024
@machikoyasuda machikoyasuda requested a review from a team as a code owner August 6, 2024 14:58
@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 and removed front-end HTML/CSS/JavaScript and Django templates labels Aug 6, 2024
Copy link

github-actions bot commented Aug 6, 2024

Coverage report

This PR does not seem to contain any modification to coverable code.

@thekaveman
Copy link
Member

@machikoyasuda just as a reminder if you're reviewing/smoke testing in prod:

image

Make sure your user agent string is overridden to cal-itp/benefits-smoke-test.

The problem: Vertical alignment of the We use the ... paragraph is off for both Desktop and Tablet.

Am I understanding that it is the left and right padding/margin around this paragraph (pushing it inwards) that is the issue? I think that is the horizontal alignment right?

@machikoyasuda
Copy link
Member Author

@thekaveman Yeah you're right - the wording is off. The horizontal spacing is off, which is taking the divs out of vertical alignment. All the divs should all start from the same point along the blue line I added in the screenshots.

@machikoyasuda machikoyasuda merged commit b073dbc into main Aug 6, 2024
13 checks passed
@machikoyasuda machikoyasuda deleted the fix/2271-card-form-bug branch August 6, 2024 19:40
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visual regression: Agency card form padding on Mobile
2 participants