Skip to content

LG-5187:Implement Improvements to focus and increase pixel size#5486

Merged
jmdembe merged 5 commits intomainfrom
jd-LG-5187-upload-focus-outline
Oct 14, 2021
Merged

LG-5187:Implement Improvements to focus and increase pixel size#5486
jmdembe merged 5 commits intomainfrom
jd-LG-5187-upload-focus-outline

Conversation

@jmdembe
Copy link
Contributor

@jmdembe jmdembe commented Oct 8, 2021

This ticket increases the pixel size of the focus outline and the outline offset

Why: To enhance the user experience for users

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this class because .usa-success-message is used in different places. So until it is decided that the checkmark will be used in all success messages, I added another class.

Copy link
Contributor

Choose a reason for hiding this comment

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

I added this class because .usa-success-message is used in different places. So until it is decided that the checkmark will be used in all success messages, I added another class.

I think we only use it within the document capture step, to my knowledge. Should be safe to update the base styles directly.

(I'm also considering this from a design system perspective, since I assume we might want to pull these styles into the design system as well)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just realized that this is in a ticket to come.

@jmdembe jmdembe force-pushed the jd-LG-5187-upload-focus-outline branch from 5bc4e19 to 8089892 Compare October 8, 2021 17:12
}
}

input:not([disabled]):focus {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we expecting this to apply to all inputs in the application?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, i did not.

@jmdembe jmdembe force-pushed the jd-LG-5187-upload-focus-outline branch 4 times, most recently from 80b091e to d3ea4ba Compare October 12, 2021 18:54
@anniehirshman-gsa
Copy link
Contributor

anniehirshman-gsa commented Oct 12, 2021

Looks good @jmdembe! One small issue I noticed testing on mobile: when you click "Take photo" and use the Acuant SDK auto-capture, the focus ring seems to "linger" behind the enable camera access modal. Any way to resolve that?

ETA: better screenshot @jmdembe @nickttng

@jmdembe
Copy link
Contributor Author

jmdembe commented Oct 13, 2021

Looks good @jmdembe! One small issue I noticed testing on mobile: when you click "Take photo" and use the Acuant SDK auto-capture, the focus ring seems to "linger" behind the enable camera access modal. Any way to resolve that?

focus_issue_mobile

This is something that is also happening in production. We can investigate this further at another time.

Comment on lines 24 to 28
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, do we need both of these sets of styles? I see the one added below would apply for an input with a value, but it also seems to apply for one without a value. Could we remove this set of styles altogether then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this can be removed. I think this was from when I was hammering away at the focus.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this offset account for the border of the input itself? For a field with a value, it seems to be much bigger an offset:

image

I can see how we'd need a big offset for the chunkier 3px border, but should it be reduced when it's the thinner border when a value is assigned? Like 4px or 5px?

@jmdembe jmdembe force-pushed the jd-LG-5187-upload-focus-outline branch from 6bf1d57 to 1d003da Compare October 14, 2021 20:07
@jmdembe jmdembe force-pushed the jd-LG-5187-upload-focus-outline branch from 1d003da to 0e6b9a5 Compare October 14, 2021 20:26
@jmdembe jmdembe merged commit 7b01caa into main Oct 14, 2021
@jmdembe jmdembe deleted the jd-LG-5187-upload-focus-outline branch October 14, 2021 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants