LG-7254 - Add 'We are retrieving' page#7090
Conversation
Why: User needs to be notified that their data is being retrieved and be given an indicator on how long it could take changelog: Internal, Inherited Proofing, We are retrieving page
holytoastr
added
the
inherited proofing
holytoastr
requested review from
gangelo,
jmhooper,
jscodefix and
rnagilla-gsa
| <div class="shield-spinner margin-y-4" id='spinner'> | ||
| <div class='text-center'> |
There was a problem hiding this comment.
Couple remarks:
- Is the ID used for anything, and if not, can we remove it?
- Can we flatten the
divs? - Is there any reason we need a custom CSS class here if we're already using the utility class to center content?
| <div class="shield-spinner margin-y-4" id='spinner'> | |
| <div class='text-center'> | |
| <div class="margin-y-4 text-center"> |
There was a problem hiding this comment.
The class was to maintain the size and styling of the gif, but I can look into alternatives if we prefer to not use use custom classes here.
There was a problem hiding this comment.
ID - removed
Divs - flattened
CSS - removed
Hopefully that addresses all your concerns!
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
Couple comments, but looks good overall 👍
| <div class='margin-y-4 text-center'> | ||
| <%= image_tag( | ||
| asset_url('shield-spinner.gif'), | ||
| alt: t('inherited_proofing.headings.retrieval'), |
There was a problem hiding this comment.
Was this text alternative provided by UX / design artifacts? I notice elsewhere we tend to use an empty text alternative for spinners, so I'm not sure what's intended to be standard practice. If anything, could be worth following-up on.
There was a problem hiding this comment.
Alt text was not provided. I can take it out if that's not standard here.
There was a problem hiding this comment.
We'll want to have the attribute (usually enforced by automated accessibility tests), but it can be set as empty to at least make it consistent with others:
| alt: t('inherited_proofing.headings.retrieval'), | |
| alt: '', |
| continue: Continuer | ||
| headings: | ||
| lets_go: to be implemented | ||
| retrieval: Nous récupérons vos informations depuis My HealtheVet. |
There was a problem hiding this comment.
The other languages don't include a period at the end of this sentence. Should they be consistent?
There was a problem hiding this comment.
I defer to the designers on this one.
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
* Add 'We are retrieving' page (LG-7254) Why: User needs to be notified that their data is being retrieved and be given an indicator on how long it could take changelog: Internal, Inherited Proofing, We are retrieving page * Adjust styling * Clean up * Update spec/views/idv/inherited_proofing/retrieval.html.erb_spec.rb Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Remove alt text from spinner Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
3 participants
This PR is for the UI only. Tasks to connect this view to the Inherited Proofing flow will follow in this sprint.
Why:
User needs to be notified that their data is being retrieved and be given an indicator on how long it could take
changelog: Internal, Inherited Proofing, We are retrieving page
🎫 Ticket
LG-7254
👀 Screenshots