-
Notifications
You must be signed in to change notification settings - Fork 113
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
LG-14700 Remove Unused CSS styling for selfie input #11313
base: main
Are you sure you want to change the base?
Conversation
[skip changelog]
Is there a screenshot of what this is intended to look like? |
Yes, adding it to screenshot section of original comment. This is also how the field looked in the previous branch. Tested locally. |
@@ -17,7 +17,7 @@ | |||
border-width: 3px; | |||
} | |||
|
|||
usa-file-input:not( | |||
.usa-file-input:not( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly are these styles meant to do? I'm having trouble figuring out what combination of states this is meant to apply to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is supposed to apply to the acuant input fields that don't have values pending and are selfie inputs (not front/back ID inputs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During initial capture and all subsequent selfie input fields during recapture if failed attempts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I don't see how these styles ever apply. I'm not seeing 21rem height on the back/front fields when I test. usa-form-group--success
is only applied if the image is changed, but then we exclude usa-file-input--has-value
so I don't see how they ever get applied.
Could you show me a screenshot where the field is rendered at 21rem height?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed up some edits to remove unneeded classes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@AShukla-GSA What's the status of this pull request? Can it be merged? |
I was asked by my team and PM to ticket this for a future sprint and keep the PR (but put a Do Not Merge Label on it). |
Any idea when that ticket might be prioritized? I'd hate to risk this falling by the wayside and forgotten about, especially when you've already done the work and the pull request is approved. |
I can bring it up to them in our next stand up, and I will make it a priority to pick it up from the backlog if free. |
[skip changelog]
🎫 Ticket
Link to the relevant ticket:
LG-14700
👀 Screenshots
If relevant, include a screenshot or screen capture of the changes.
After: