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

EP-2354 - branded checkout changes #1121

Open
wants to merge 15 commits into
base: EP-2517-branded-checkout-improvements
Choose a base branch
from

Conversation

wjames111
Copy link

Primary task highlighted in JIra ticket EP-2527, Update Name of Card # to Card Number. Update includes layout changes to the standard checkout, which we'll need to decide whether to make this change across the board or specifically on the new version.

@wjames111 wjames111 self-assigned this Nov 26, 2024
@wjames111 wjames111 force-pushed the EP-2354-branded-checkout-changes branch from a71f602 to d7840e0 Compare November 26, 2024 20:54
@wjames111
Copy link
Author

@wrandall22 does this look like what you were asking? I removed logic and references for hideSpouseDetails and left showSpouseDetails toggle button and logic.

@wjames111 wjames111 added the On Staging Will be merged to the staging branch by Github Actions label Jan 31, 2025
@wrandall22
Copy link
Contributor

I believe there is some JS logic around the button as well.

@wjames111
Copy link
Author

Oh my mistake, I thought we were leaving the showSpouseDetails toggle button in there.

Copy link
Contributor

@wrandall22 wrandall22 left a comment

Choose a reason for hiding this comment

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

Something about the layout is a little off, the vertical spacing between city and state/zip is not the same. Also, we're missing the required field styling on the state/zip for some reason.

Screenshot 2025-02-11 at 8 56 27 AM

README.md Outdated Show resolved Hide resolved
@@ -11,7 +11,7 @@
</head>
<body>

<branded-checkout designation-number="2294554" show-cover-fees="true" use-v3="true" ng-cloak></branded-checkout>
<branded-checkout designation-number="2294554" show-cover-fees="true" use-v3="true" ng-cloak hide-spouse-details="true" hide-annual="true" hide-quarterly="true"></branded-checkout>
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not commit this change.

<div ng-if="$ctrl.compactAddress">
<div class="col-sm-3">
<div class="form-group is-required" ng-class="{'has-error': ($ctrl.parentForm.addressRegion | showErrors) || $ctrl.loadingRegionsError}">
<label translate>{{'STATE'}}</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update the label structure to match the non-compact address version for accessibility?

@wrandall22
Copy link
Contributor

Between 550px and 710px, the labels are too long (not sure how many devices fit into this range, but I see the iPhone SE 2nd Gen seems to)

Screenshot 2025-02-11 at 9 58 20 AM

@wjames111
Copy link
Author

@wrandall22 what are you thinking for label length? We could scale down all label sizes, remove additional label names or we could add nowrap or something (probably a bad idea)

@wrandall22
Copy link
Contributor

One option would be to use col-sm-12 col-md-6 instead of col-sm-6. This would put it on one line for screen sizes above 992px but separate lines for anything smaller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
On Staging Will be merged to the staging branch by Github Actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants