-
Notifications
You must be signed in to change notification settings - Fork 0
USE-316: Updated Primo timeout message and styling #338
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
Conversation
Pull Request Test Coverage Report for Build 21033633891Details
💛 - Coveralls |
matt-bernhardt
left a comment
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.
I don't know if this is a "request changes" or just a concern because I'm failing to review this accurately - but I don't see the icon I think you're intending in the alert? I've set the review app to throw the error very quickly (the threshold is a tenth of a second), and I've been able to see the error pretty consistently that way.
|
|
||
| &:before{ | ||
| color: $color-icon-danger; | ||
| content: "\f05e"; |
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.
Ahh fascinating... This problem happened when working on Wordpress... I have the icon font locally so it appeared on my local environment, but I must not be referencing it correctly for the version we have on our CDN. I'll update this!
|
@matt-bernhardt Can you check the review app again for me? I updated the reference (I put a reference to version 6 instead of 7. 7 is the one included in USE...). It looks good on my end, even when disabling local font rendering in chrome inspect, but would love someone without the local font to confirm. |
matt-bernhardt
left a comment
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.
Looks good! Thanks.


Developer
Previously, we had a message when Primo timed out that wasn't as user friendly as we'd prefer. This work updates that message and adjusts the styling to be closer to our design system component for Alert.
This required some new semantic color variables.
This work does NOT adjust the other flavors of Alert. Since they're probably shared with GeoData, it seems wise to make as few changes as possible until we can investigate all of the tendrils.
Accessibility
New ENV
Approval beyond code review
Additional context needed to review
E.g., if the PR includes updated dependencies and/or data
migration, or how to confirm the feature is working.
Code Reviewer
Code
added technical debt.
Documentation
(not just this pull request message).
Testing